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

[Daniel D. Daugherty]

Adding hotspot-jfr-dev at ... since this bug is filed against hotspot/jfr...

Dan


On 5/12/20 2:33 AM, Thomas Stüfe 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