On 2020-01-18 17:51, Stuart Marks wrote:
On 1/16/20 8:21 AM, Claes Redestad wrote:
I've cleaned up a bit (SALT can be made into a long to reduce casts
etc), and rolled it into the current patch:
http://cr.openjdk.java.net/~redestad/8236641/open.01/
OK, I think this makes sense. But I did have to stare at the code for a
while to figure out what was going on. So, maybe a couple judicious
comments would be helpful.
68 SALT32L = (int)((color * seed) >> 16) & 0xFFFF_FFFFL; //
avoid LSB and MSB
The comment "avoid LSB and MSB" isn't helping me here, and in fact it's
confusing because I thought it applied to the "& 0xFFFF_FFFFL" part. Is
there something else that ought to be said here, perhaps "select the
middle 32 bits of the product"?
770 idx = (int) ((SALT32L * elements.length) >>> 32);
1063 idx = (int) ((SALT32L * (table.length >> 1)) >>>
32) << 1;
These expressions replace floorMod(), which (at least to my brain)
idiomatically ensures the return value is in the range [0..len-1]. It's
not at all obvious what these do, although I think they do the same
thing. Can we add a comment something like, "add a 'random' starting
index based on the SALT"?
Thanks for reviewing!
Uploaded new version with improved comments here:
http://cr.openjdk.java.net/~redestad/8236641/open.01/
/Claes
