RFR: 8351565: Implement JEP 502: Stable Values (Preview)

[Per Minborg]

On Tue, 11 Mar 2025 07:50:38 GMT, Quan Anh Mai <qamai at openjdk.org> wrote:

>> Implement JEP 502.
>> 
>> The PR passes tier1-tier3 tests.
>
> src/java.base/share/classes/java/util/ImmutableCollections.java line 777:
> 
>> 775:         private final IntFunction<? extends E> mapper;
>> 776:         @Stable
>> 777:         private final StableValueImpl<E>[] backing;
> 
> You can use a backing `@Stable Object[]` instead. It will reduce indirection when accessing this list.

Can you please elaborate a bit more on your proposal @merykitty?

> src/java.base/share/classes/jdk/internal/lang/stable/StableValueImpl.java line 65:
> 
>> 63:     //
>> 64:     @Stable
>> 65:     private volatile Object value;
> 
> Can we use `acquire`/`release` semantics instead of `volatile`?

Yes we can. However, I am uncertain if the added complexity can motivate any performance benefits. Perhaps on ARM? I can do a benchmark on it.

> src/java.base/share/classes/jdk/internal/lang/stable/StableValueImpl.java line 128:
> 
>> 126:             final T newValue = supplier.get();
>> 127:             // The mutex is reentrant so we need to check if the value was actually set.
>> 128:             return wrapAndCas(newValue) ? newValue : orElseThrow();
> 
> Reentrancy into here seems really buggy, I would endorse disallowing it instead. In that case, a `ReentrantLock` seems better than the native monitor as we can cheaply check `lock.isHeldByCurrentThread()`

StableValueImpl was carefully designed to minimize memory footprint. Adding a lock would inflate memory usage substantially.

> src/java.base/share/classes/jdk/internal/lang/stable/StableValueImpl.java line 159:
> 
>> 157:     private boolean wrapAndCas(Object value) {
>> 158:         // This upholds the invariant, a `@Stable` field is written to at most once
>> 159:         return UNSAFE.compareAndSetReference(this, UNDERLYING_DATA_OFFSET, null, wrap(value));
> 
> There is no need for a cas here as all setters have to hold the lock. We should have a dedicated private `set` that asserts `Thread.holdsLock(this)`.

This is more of a belt and suspenders solution. It is true that it is redundant. A set volatile would suffice here.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/23972#discussion_r1989016337
PR Review Comment: https://git.openjdk.org/jdk/pull/23972#discussion_r1988630199
PR Review Comment: https://git.openjdk.org/jdk/pull/23972#discussion_r1993142117
PR Review Comment: https://git.openjdk.org/jdk/pull/23972#discussion_r1988634451