RFR: 8236641: Improve Set.of(...).iterator() warmup characteristics

[Stuart Marks]

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