RFR: 8221836: Avoid recalculating String.hash when zero

[Andrew Dinn]

On 09/04/2019 09:42, Claes Redestad wrote:
> Hi Andrew,
> 
> On 2019-04-09 10:20, Andrew Dinn wrote:
>> If the patch is to go in -- and I concede there is an acceptable
>> argument for that -- then it
>> needs commenting to help avoid this happening.
> 
> open.02 already adds what I believed to be sufficient commenting, have
> you taken this into consideration?

     public int hashCode() {
+        // The hash or hashIsZero fields are subject to a benign data race,
+        // making it crucial to ensure that any observable result of the
+        // calculation in this method stays correct under any possible
read of
+        // these fields. One necessary restriction to allow this to be
correct
+        // without explicit memory fences or similar concurrency
primitives is
+        // that we can ever only write to one of these two fields for a
given
+        // String instance.
         int h = hash;

It would probably also be good if you extended the comment to document
the status quo i.e. as Peter noted that the assigned values are computed
deterministically from immutable state. Perhaps this:

     public int hashCode() {
+        // The hash or hashIsZero fields are subject to a benign data race,
+        // making it crucial to ensure that any observable result of the
+        // calculation in this method stays correct under any possible
read of
+        // these fields. One necessary restriction to allow this to be
correct
+        // without explicit memory fences or similar concurrency
primitives is
+        // that we can ever only write to one of these two fields for a
given
+        // String instance. Clearly the assigned values must also be
computed
+        // deterministically from immutable state.
         int h = hash;

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