optimize KlassInfoTable size to power of 2

[臧琳]

Thanks Paul for looking into this!

Hi All,
       Can I get more review about this:
Webrev: http://cr.openjdk.java.net/~jcbeyler/8215228/webrev.00/
Bug: https://bugs.openjdk.java.net/browse/JDK-8215228
Thanks.

BRs,
Lin

From: Hohensee, Paul [mailto:hohensee at amazon.com]
Sent: Wednesday, December 12, 2018 10:08 PM
To: 臧琳 <zanglin5 at jd.com>; JC Beyler <jcbeyler at google.com>
Cc: serviceability-dev at openjdk.java.net
Subject: Re: optimize KlassInfoTable size to power of 2

Lgtm, JC.

Thanks,

Paul

From: serviceability-dev <serviceability-dev-bounces at openjdk.java.net<mailto:serviceability-dev-bounces at openjdk.java.net>> on behalf of 臧琳 <zanglin5 at jd.com<mailto:zanglin5 at jd.com>>
Date: Tuesday, December 11, 2018 at 6:31 PM
To: JC Beyler <jcbeyler at google.com<mailto:jcbeyler at google.com>>
Cc: "serviceability-dev at openjdk.java.net<mailto:serviceability-dev at openjdk.java.net>" <serviceability-dev at openjdk.java.net<mailto:serviceability-dev at openjdk.java.net>>
Subject: RE: optimize KlassInfoTable size to power of 2

Dear JC,
       That’s exactly modification I made ☺
       Thanks a lot for making the patch!

BRs,
Lin




From: JC Beyler [mailto:jcbeyler at google.com]
Sent: Wednesday, December 12, 2018 2:27 AM
To: 臧琳 <zanglin5 at jd.com<mailto:zanglin5 at jd.com>>
Cc: Andrew Haley <aph at redhat.com<mailto:aph at redhat.com>>; serviceability-dev at openjdk.java.net<mailto:serviceability-dev at openjdk.java.net>
Subject: Re: optimize KlassInfoTable size to power of 2

Hi all,

I was playing with this yesterday because I was curious to see the different code generation patterns so I had the webrev ready to go:

Webrev: http://cr.openjdk.java.net/~jcbeyler/8215228/webrev.00/
Bug: https://bugs.openjdk.java.net/browse/JDK-8215228

@Lin: check but I'd imagine this is what you had originally done but you also changed the size

Thanks,
Jc




On Tue, Dec 11, 2018 at 3:44 AM 臧琳 <zanglin5 at jd.com<mailto:zanglin5 at jd.com>> wrote:

Oh, to make it clear, when testing with 65536, I made the change to use hash & 65535
instead of hash % 65536.


BRs,
Lin



-----Original Message-----
From: 臧琳
Sent: Tuesday, December 11, 2018 7:36 PM
To: 'Andrew Haley' <aph at redhat.com<mailto:aph at redhat.com>>; serviceability-dev at openjdk.java.net<mailto:serviceability-dev at openjdk.java.net>
Subject: RE: optimize KlassInfoTable size to power of 2

>There is something that I do not understand. You said that GCC didn't
>know that _num_buckets was constant. In  that case, how did GCC know not to use a divide instruction when you tried 65536?

The original code doesn't use _num_buckets directly when calculating the hash idx, it use a non-constant variable _size, a member of KlassInfoTable, which is assigned to _num_buckets. I guess it is the reason GCC didn't do the constant propagation in this case. And _size is not changed during the life of KlassInfoTable, it only set to zero at de-construction.

In my experiment, I use _num_buckets directly instead of _size to calculate hash idx. So GCC provides the optimizations.

I think your suggestion of using 65537 is valuable, And I agree that it is not easy to prove that non-prime for hash is better. So how about make the change to use constant _number_buckets directly for calculating hash idx, and leave the prime not changed. In my case, speed up JMap histo helps at avoid getting killed by a timer, especially when heap is large at ~200GB.

Do you think it is reasonable to have a patch?

Thanks,
Lin

-----Original Message-----
From: Andrew Haley [mailto:aph at redhat.com<mailto:aph at redhat.com>]
Sent: Tuesday, December 11, 2018 7:10 PM
To: 臧琳 <zanglin5 at jd.com<mailto:zanglin5 at jd.com>>; serviceability-dev at openjdk.java.net<mailto:serviceability-dev at openjdk.java.net>
Subject: Re: optimize KlassInfoTable size to power of 2

On 12/11/18 11:05 AM, Andrew Haley wrote:
> unsigned mod_m(unsigned n) {
>   unsigned tmp = n % 65536;
>   tmp -= n / 65536;
>   if (tmp >= 65537) // overflow
>     tmp += 65537;
>   return tmp;
> }

NB: this assumes that unsigned int is uint64_t.

--
Andrew Haley
Java Platform Lead Engineer
Red Hat UK Ltd. <https://www.redhat.com>
EAC8 43EB D3EF DB98 CC77 2FAD A5CD 6035 332F A671


--

Thanks,
Jc
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/serviceability-dev/attachments/20181213/0c110a0a/attachment-0001.html>