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
