On 1/13/20 8:03 AM, Claes Redestad wrote:
we're doing plenty of iterations over Set.of() instances during
bootstrap, which makes these operations show up prominently in
startup profiles. This patch refactors a few hot methods to get a
measureable startup improvement without regressing on targeted
microbenchmarks.
Bug: https://bugs.openjdk.java.net/browse/JDK-8236641
Webrev: http://cr.openjdk.java.net/~redestad/8236641/open.00/
OK, interesting and promising. A potential pitfall and a nitpick, though perhaps
an important one.
65 static {
66 long color = 0x243F_6A88_85A3_08D3L; // pi slice
67 long seed = System.nanoTime();
68 int salt = (int)((color * seed) >> 16); // avoid LSB and MSB
69 REVERSE = salt >= 0;
70 // Force SALT to be positive so we can use modulo rather
71 // than Math.floorMod
72 SALT = Math.abs(salt);
73 }
After this code runs I'm not convinced that SALT will be positive... consider
Integer.MIN_VALUE! Maybe just mask off the top bit instead? Thirty-one bits of
salt should be sufficient for our purposes here.
In SetNIterator::next there is this code:
789 do {
790 if (REVERSE) {
791 if (++idx >= len) {
792 idx = 0;
793 }
794 } else {
795 if (--idx < 0) {
796 idx = len - 1;
797 }
798 }
799 }
800 while ((element = elements[idx]) == null);
Ah, the rare do-while loop! I think this makes sense, but it reads oddly,
because it looks like the trailing "while" starts a new statement when in fact
it's part of the do-while statement. The trailing semicolon then makes this look
like a mistake. I'd suggest merging 799 and 800 so the last line of the loop
looks like this:
799 } while ((element = elements[idx]) == null);
It's a small thing, but the "} while" on a line should be a tip-off that this is
the rare do-while loop and not something else.
Thanks,
s'marks
