RFR: 8351565: Implement JEP 502: Stable Values (Preview) [v25]

Wed Apr 2 09:45:52 UTC 2025

On Tue, 1 Apr 2025 13:27:34 GMT, Per Minborg <pminborg at openjdk.org> wrote:

>> Implement JEP 502.
>> 
>> The PR passes tier1-tier3 tests.
>
> Per Minborg has updated the pull request incrementally with two additional commits since the last revision:
> 
>  - Add lazy toSting for StableMap::values
>  - Make toString for reversed and sublist lazy

src/java.base/share/classes/java/lang/StableValue.java line 257:

> 255:  * which are held by stable values:
> 256:  * {@snippet lang = java:
> 257:  * public final class DependencyUtil {

I think the fibonacci example is great -- and is probably enough for this section. We also had, at some point, a nice ASCII art of the computations involved in the fibonacci, which might be useful to illustrate your claim re. "dependency graph". Do you think you can recover that ?

src/java.base/share/classes/java/lang/StableValue.java line 327:

> 325:  * (e.g. {@linkplain #trySet(Object) trySet()})
> 326:  * {@linkplain java.util.concurrent##MemoryVisibility <em>happens-before</em>}
> 327:  * any subsequent read operation (e.g. {@linkplain #orElseThrow()}).

Maybe we should clarify that a write of V happens before a read that can see that can see that V. E.g. a successful `trySet` doesn't happen before _any_ `orElseThrow` _in general_ -- (because `orElseThrow` can also throw -- in which case you know you arrived too early). Also, there's operations in there like `orElseSet` which both get and set -- so perhaps things should be specified a bit more -- e.g. maybe we should talk about a successful read operation as:

* `orElseThrow` that doesn't throw
* `orElse` which dosn't return the fallback value
* `orElseGet` that doesn't run the provided supplier

We can also talk about a succseful write operation:

* a `trySet` which returns true
* a `setOrThrow` that doesn't throw
* an `orElseSet` that runs the supplier

Then we can say that a _successful_ write operation happens before a _successful_ read operation.

src/java.base/share/classes/java/lang/StableValue.java line 371:

> 369: 
> 370:     /**
> 371:      * {@return {@code true} if the content was set to the provided {@code value},

I find this description a bit confusing -- as written it almost looks like a predicate -- e.g. test if this stable value is set with the value I'm providing. But the important thing here is the side-effect -- so I feel the javadoc for this method should be much less about what this method returns, and much more about what the method actually does. (`setOrThrow` seems to be better in this respect)

src/java.base/share/classes/java/lang/StableValue.java line 378:

> 376:      * @param value to set
> 377:      */
> 378:     boolean trySet(T value);

This can probably throw `IllegalStateException` too right -- e.g. if used inside a supplier passed to `orElseSet` ?

src/java.base/share/classes/java/lang/StableValue.java line 416:

> 414:      * When this method returns successfully, the content is always set.
> 415:      * <p>
> 416:      * This method will always return the same witness value regardless if invoked by

will always return/always returns (more direct)

src/java.base/share/classes/java/lang/StableValue.java line 435:

> 433:      *
> 434:      * @param value to set
> 435:      * @throws IllegalStateException if the content was already set

The javadoc speaks about "a stable value is set", and not "content is set". Make sure we're using terminology consistenty

src/java.base/share/classes/java/lang/StableValue.java line 461:

> 459:      * An unset stable value has no content.
> 460:      * <p>
> 461:      * The returned stable value is not {@link Serializable}.

I find the claim about serializable a bit odd here. Perhaps in some `implSpec`/`apiNote` at the toplevel?

src/java.base/share/classes/java/lang/StableValue.java line 507:

> 505:      * @param <T>      the type of results supplied by the returned supplier
> 506:      */
> 507:     static <T> Supplier<T> supplier(Supplier<? extends T> original) {

I think that here and in all other functional/collection factories, we should document that `toString` will never cause a value to be computed.

src/java.base/share/classes/java/lang/StableValue.java line 591:

> 589:      * <p>
> 590:      * The returned list is an {@linkplain Collection##unmodifiable unmodifiable} list
> 591:      * whose size is known at construction. The list's elements are computed via the

s/"is known at construction"/`{@code size}` ?

src/java.base/share/classes/java/lang/StableValue.java line 603:

> 601:      * caller and no value for the element is recorded.
> 602:      * <p>
> 603:      * The returned list and its {@link List#subList(int, int) subList} views implement

Perhaps the most important fact to note here is that the returned lists via `subList` and friends are _also_ stable lists!

src/java.base/share/classes/java/lang/StableValue.java line 657:

> 655:      * @param <V>    the type of mapped values in the returned map
> 656:      */
> 657:     static <K, V> Map<K, V> map(Set<K> keys,

Like `List`, a map can also generate other kinds of collections:
1. an entry set (`Map::entrySet`)
2. a key set (`Map::keySet`)
3. a value set (`Map::values`)

We should make it clear that calling (1) and (3) will result in materializing the values for the map -- we do not have "lazy" views for such sets.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/23972#discussion_r2024400960
PR Review Comment: https://git.openjdk.org/jdk/pull/23972#discussion_r2024413931
PR Review Comment: https://git.openjdk.org/jdk/pull/23972#discussion_r2024425037
PR Review Comment: https://git.openjdk.org/jdk/pull/23972#discussion_r2024438670
PR Review Comment: https://git.openjdk.org/jdk/pull/23972#discussion_r2024431337
PR Review Comment: https://git.openjdk.org/jdk/pull/23972#discussion_r2024442624
PR Review Comment: https://git.openjdk.org/jdk/pull/23972#discussion_r2024445136
PR Review Comment: https://git.openjdk.org/jdk/pull/23972#discussion_r2024469913
PR Review Comment: https://git.openjdk.org/jdk/pull/23972#discussion_r2024451560
PR Review Comment: https://git.openjdk.org/jdk/pull/23972#discussion_r2024454122
PR Review Comment: https://git.openjdk.org/jdk/pull/23972#discussion_r2024461588