RFR: 8330465: Stable Values and Collections (Internal) [v5]

[ExE Boss]

On Wed, 15 May 2024 16:10:06 GMT, Chen Liang <liach at openjdk.org> wrote:

>> Per Minborg has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Switch to monomorphic StableValue and use lazy arrays
>
> src/java.base/share/classes/jdk/internal/lang/stable/StableValueImpl.java line 104:
> 
>> 102:         // Optimistically try plain semantics first
>> 103:         final V v = value;
>> 104:         if (v != null) {
> 
> If `value == null && state == NULL`, can the path still be constant folded? I doubt it because `value` in this case may not be promoted to constant.

Maybe the `state == NULL` check should be moved before `v != null`, as the **JIT** doesn’t constant‑fold `null` [`@Stable`] values:
https://github.com/openjdk/jdk/blob/8a4315f833f3700075d65fae6bc566011c837c07/src/java.base/share/classes/jdk/internal/vm/annotation/Stable.java#L41-L44
https://github.com/openjdk/jdk/blob/8a4315f833f3700075d65fae6bc566011c837c07/src/java.base/share/classes/jdk/internal/vm/annotation/Stable.java#L64-L71

[`@Stable`]: https://github.com/openjdk/jdk/blob/master/src/java.base/share/classes/jdk/internal/vm/annotation/Stable.java

> src/java.base/share/classes/jdk/internal/lang/stable/StableValueImpl.java line 240:
> 
>> 238:                 }
>> 239:             } finally {
>> 240:                 supplying = false;
> 
> Resetting a stable field is a bad idea. I recommend renaming this to `supplierCalled` or `supplied` so we never transition this false -> true

Yes, according to the `@Stable` annotation’s JavaDoc, this is UB:
https://github.com/openjdk/jdk/blob/8a4315f833f3700075d65fae6bc566011c837c07/src/java.base/share/classes/jdk/internal/vm/annotation/Stable.java#L74-L80

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

PR Review Comment: https://git.openjdk.org/jdk/pull/18794#discussion_r1602101301
PR Review Comment: https://git.openjdk.org/jdk/pull/18794#discussion_r1602106099