RFR: 8221836: Avoid recalculating String.hash when zero

[Andrew Dinn]

On 08/04/2019 10:42, Claes Redestad wrote:
> On 2019-04-08 11:35, Aleksey Shipilev wrote:
>>> Sure, String::hashCode/hash_code locally becomes a bit more complex, but
>>> I view this as being a net improvement on the total amount of special
>>> handling we need to do for Strings and their hash codes.
>> I don't see it. The change *added* new handling for the flag in all
>> those places we used to handle
>> zero hash code, and then some.
> 
> There's a few simple boilerplate methods added and the logic of
> hash_code(string) is consolidated to mimic String::hashCode, but code at
> the real call-sites like stringDedupTable and stringTable is simplified.
Aleksey, I'm definitely buying Claes argument on this point. Also, I
think your other quibble suffers from "what-aboutism" -- the fact that
there are other ways for perverse performance issues to manifest
(hashcode collisions) doesn't mean that this gap should not be plugged.

However, you also said in your opening criticism

  "I had hard time convincing myself that code is concurrency-safe"

I think that is a more telling complaint. Can you elaborate on why you
found it hard to convince yourself of this? (I know what I think is the
issue and I don't view it as an /especially/ thorny problem).

Claes, is there a reason why you named the argument to method
hash_is_set 'string' when every other method uses the name
'java_string'? Is you 'j' key a tad sticky?

regards,


Andrew Dinn
-----------
Senior Principal Software Engineer
Red Hat UK Ltd
Registered in England and Wales under Company Registration No. 03798903
Directors: Michael Cunningham, Michael ("Mike") O'Neill, Eric Shander