On Tue, 1 Jul 2025 08:21:10 GMT, Andrew Haley <a...@openjdk.org> wrote:

>> src/java.base/share/classes/java/lang/ScopedValue.java line 802:
>> 
>>> 800:                         return x;
>>> 801:                     }
>>> 802:                 };
>> 
>> There's something a bit uncomfortable about initializing hashGenerator in 
>> ScopedValue's class initializer, then changing it in Constants class 
>> initializer. Iimmediately clear what the memory model issues. It doesn't of 
>> course matter if the stale value is used but I think confusing on first 
>> sight.
>> 
>> Have you tried dropping the initializer from ScopedValue so it hashGenerator 
>> is only set by Constants? The ScopedValue constructor can call generateKey 
>> if hashGenerator is null, and hashGenerator can become a stable field.
>
>> There's something a bit uncomfortable about initializing hashGenerator in 
>> ScopedValue's class initializer, then changing it in Constants class 
>> initializer. Iimmediately clear what the memory model issues. It doesn't of 
>> course matter if the stale value is used but I think confusing on first 
>> sight.
> 
> That's fair. I think it's a benign race, but I guess that's still a race.
> 
>> Have you tried dropping the initializer from ScopedValue so it hashGenerator 
>> is only set by Constants? The ScopedValue constructor can call generateKey 
>> if hashGenerator is null, and hashGenerator can become a stable field.
> 
> That would be more explicit.

> > Have you tried dropping the initializer from ScopedValue so hashGenerator 
> > is only set by Constants? The ScopedValue constructor can call generateKey 
> > if hashGenerator is null, and hashGenerator can become a stable field.
> 
> That would be more explicit.

A stable field would also be constant foldable.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/26019#discussion_r2176927586

Reply via email to