RFR(xs): 8244777: ClassLoaderStats VM Op uses constant hash value

[Thomas Stüfe]

Ping.

This is a trivial fix. Could I have another review please?

All tests ran through. I tested the hash function manually and it works
fine. The hash algorithm is a standard one, see e.g. zHash.hpp.

Thanks, Thomas

On Tue, May 12, 2020 at 8:33 AM Thomas Stüfe <thomas.stuefe at gmail.com>
wrote:

> Hi,
>
> may I get reviews for this small fix:
>
> bug: https://bugs.openjdk.java.net/browse/JDK-8244777
> webrev:
> http://cr.openjdk.java.net/~stuefe/webrevs/8244777--classloaderstats-vm-op-uses-constant-hash-value/webrev.00/webrev/
>
> This fixes runtime issues with the ClassLoaderStats VM Operation - with
> 50K classloaders it currently takes 1.5s, with this patch it takes ~13 ms.
> This issue was reported at [1].
>
> ClassLoaderStats iterates the CLDG and accumulates statistics per class
> loader over all CLDs. It uses a temporary ResourceHashTable for that.
>
> The hash function is wrong though. It has two errors which independently
> from each other cause the hash value to be constant, so the
> ResourceHashTable degenerates to a linked list.
>
> static unsigned oop_hash(oop const& s1) {
>    unsigned hash = (unsigned)((uintptr_t)&s1);
>    return hash ^ (hash >> LogMinObjAlignment);
> }
>
> - we use &s1, so we calculate the hash based on the address of the key
> carrier not the key, which is always the same
> - we use LogMinObjAlignment instead of LogMinObjAlignmentInBytes, which is
> usually 0, so we xor out the hash at the end.
>
> After playing with the corrected version I got better results; then I
> swapped this implementation with a common integer hash function we also use
> in zgc, and got even better results. For a hash table with 256 buckets and
> 50000 entries I now get a max chain len of 230, which is pretty close to
> the optimal ~200.
>
> Thanks, Thomas
>
> [1]
> https://mail.openjdk.java.net/pipermail/hotspot-jfr-dev/2020-May/001430.html
>
>
>