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

Quan Anh Mai qamai at openjdk.org
Thu Mar 13 11:20:14 UTC 2025


On Mon, 10 Mar 2025 18:11:23 GMT, Per Minborg <pminborg 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.

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`?

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()`

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)`.

src/java.base/share/classes/jdk/internal/lang/stable/StableValueImpl.java line 168:

> 166:     // Wraps `null` values into a sentinel value
> 167:     @ForceInline
> 168:     private static <T> T wrap(T t) {

Suggestion:

    private static <T> Object wrap(T t) {

src/java.base/share/classes/jdk/internal/lang/stable/StableValueImpl.java line 181:

> 179:     @SuppressWarnings("unchecked")
> 180:     @ForceInline
> 181:     private static <T> T nullSentinel() {

Suggestion:

    private static Object nullSentinel() {

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

PR Review Comment: https://git.openjdk.org/jdk/pull/23972#discussion_r1988608920
PR Review Comment: https://git.openjdk.org/jdk/pull/23972#discussion_r1988612784
PR Review Comment: https://git.openjdk.org/jdk/pull/23972#discussion_r1993081551
PR Review Comment: https://git.openjdk.org/jdk/pull/23972#discussion_r1988616943
PR Review Comment: https://git.openjdk.org/jdk/pull/23972#discussion_r1993110162
PR Review Comment: https://git.openjdk.org/jdk/pull/23972#discussion_r1993111723


More information about the core-libs-dev mailing list