RFR: 8221836: Avoid recalculating String.hash when zero

[Aleksey Shipilev]

On 4/8/19 12:15 PM, Andrew Dinn wrote:
> 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.

Again, I don't see it. The same cleanup (moving hash computation code to java_lang_String::hash*)
can be done without introducing the flag?

> 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.

Read carefully: I said that alternative hashing that is there to mitigate hashcode collisions *also*
takes care of zero hashcode attacks. So if we do care about obscure zero hashcode attack, we can
piggyback on the already implemented mechanism that is there to mitigate the much broader attack.


> 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

Because the whole thing in current code is "benign data race" on hash field. Pulling in another
field into race needs careful consideration if it breaks the benignity. It apparently does not, but
the cognitive complexity involved in reading that code makes the minuscule benefit much more
questionable.

-Aleksey

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <https://mail.openjdk.org/pipermail/hotspot-gc-dev/attachments/20190408/f4462908/signature.asc>