Dear JC, That’s exactly modification I made ☺ Thanks a lot for making the patch!
BRs, Lin From: JC Beyler [mailto:jcbey...@google.com] Sent: Wednesday, December 12, 2018 2:27 AM To: 臧琳 <zangl...@jd.com> Cc: Andrew Haley <a...@redhat.com>; serviceability-dev@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 臧琳 <zangl...@jd.com<mailto:zangl...@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' <a...@redhat.com<mailto:a...@redhat.com>>; serviceability-dev@openjdk.java.net<mailto:serviceability-dev@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:a...@redhat.com<mailto:a...@redhat.com>] Sent: Tuesday, December 11, 2018 7:10 PM To: 臧琳 <zangl...@jd.com<mailto:zangl...@jd.com>>; serviceability-dev@openjdk.java.net<mailto:serviceability-dev@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