RFR: 8253089: Windows (MSVC 2017) build fails after JDK-8243208

Aleksey Shipilev shade at redhat.com
Mon Sep 14 17:27:36 UTC 2020


On 9/14/20 6:05 PM, Kim Barrett wrote:
> I think changing to using u2 isn't the right solution though.  This throws
> away a lot of bits in the hash-code calculation, potentially making it less
> effective.  

I think this accuracy concern does not apply here, because it does not actually change the computation.

This is because polynomial hash codes enjoy the modulo distributivity, computation uses unsigned 
(non-negative) values, and the fact that the result is finally stored in u2, cutting out whatever 
upper bits hash code had accumulated.

A bit more rigorously:

   M = 2^(sizeof(u2)*8) = 16
   K = 2^(sizeof(unsigned int)*8) = 32  // assume int is 32 bit

   L1: note M and K are power of two, and K=2*M, so (x % K) % M = (x % M) here
     (Translation: cutting out K lower bits, and then M (M < K) lower bits is equivalent to
                   cutting out M lower bits right away)

   new_hashcode = sum_i( (31^k * s[i]) % M) % M

   old_hashcode = sum_i( (31^k * s[i]) % K) % M
                = sum_i(((31^k * s[i]) % K) % M) % M // modulo distributivity
                = sum_i( (31^k * s[i]) % M) % M      // by L1
                = new_hashcode

Or try this ;)
   https://cr.openjdk.java.net/~shade/8253089/mod-hashcode.cpp

> I think a better workaround is to locally suppress the warning,
> which I think is from the multiplication here:
> 
> 61       h = 31*h + (unsigned int) *s;
> 
> So I suggest wrapping that line with targeted warning suppression, i.e.
> using PRAGMA_DIAG_PUSH/POP and PRAGMA_DISABLE_MSVC_WARNING(4307) with an
> appropriate comment. I don't love cluttering code with that kind of thing,
> but I think in this case that's a better solution than changing what's
> being calculated.

I think math thankfully saves us from cluttering the code with compiler pragmas.

-- 
Thanks,
-Aleksey



More information about the hotspot-runtime-dev mailing list