RFR: 8221723: Avoid storing zero to String.hash

[Claes Redestad]

Hi Aleksey,

On 2019-04-01 15:03, Aleksey Shipilev wrote:
> On 4/1/19 1:57 PM, Claes Redestad wrote:
>> when a String has a calculated hash code value of 0, we recalculate and
>> store a 0 to the String.hash field every time (except for the empty
>> String, which is special cased). To make String objects more amenable to
>> storage in shared read-only memory, e.g., CDS archives, we should avoid
>> this redundant store.
>>
>> Bug:    https://bugs.openjdk.java.net/browse/JDK-8221723
>> Webrev: http://cr.openjdk.java.net/~redestad/8221723/
> 
> Looks fine. I recall we were fixing something like this, but I might be confusing it with the other
> RFE [0].

digging up the history I did a regression fix due some benchmarks
regressing during JDK 9:
http://hg.openjdk.java.net/jdk9/jdk9/jdk/rev/6837759aa403 (closed bug
due some clerical errors linking to internal systems..)

Cause of regression was https://bugs.openjdk.java.net/browse/JDK-8058643

The approach of avoiding the store for any String was however lost when
the JEP work for Compact Strings was integrated, and since the
performance benefit for anything by "" is comparatively slim I didn't
insist on fixing this at the time. With the added motivation that it
helps storing Strings on read-only memory it feels worthwhile, though.

> The comment might emphasize we are doing it for RO memory:
> 
>     // Avoid issuing a store if the calculated value is also zero:
>     // in addition to minor optimization benefit, this also allows storing
>     // Strings with zero hash code in read-only memory.

Yes, that reads better and better emphasizes the point I'm trying to
make. Updated in place.

Thanks!

/Claes