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

[Thomas Stüfe]

Hi Colleen,

I use this function for many years in my coding, so it has proven its
value. It stems from this work:

http://web.archive.org/web/20071223173210/http://www.concentric.net/~Ttwang/tech/inthash.htm

it is based on a proposal by one Robert Jenkins. Only after adding it I
discovered Zgc using the same algorithm.

Making primitive_hash() this function would be probably good but I'd prefer
to do this in a separate RFE if possible.

Thanks, Thomas


On Wed, May 13, 2020 at 9:06 PM <coleen.phillimore at oracle.com> wrote:

>
>
> On 5/13/20 2:39 PM, coleen.phillimore at oracle.com wrote:
> >
> > Hi, This looks good but where is this hash function from (without
> > searching the internet for "* 2057")?
>
> I found it: https://gist.github.com/badboy/6267743
>
> It doesn't have seem to have a fancy name.
>
> Looks good,
> Coleen
> >
> > Maybe primitive_hash should be this hash?  I'm not suggesting changing
> > it at this time, just put a comment on what this hash is called if it
> > has a name.
> >
> > thanks,
> > Coleen
> >
> > On 5/13/20 2:08 PM, Thomas Stüfe wrote:
> >> 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
> >>>
> >>>
> >>>
> >>>
> >
>
>