On Wed, 15 May 2024 16:10:06 GMT, Chen Liang <[email protected]> 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
