RFR: 8366178: Implement JEP 526: Lazy Constants (Second Preview) [v25]

Jorn Vernee jvernee at openjdk.org
Wed Nov 12 16:39:57 UTC 2025 

On Wed, 12 Nov 2025 10:07:39 GMT, Per Minborg <pminborg at openjdk.org> wrote:

>> Implement JEP 526: Lazy Constants (Second Preview)
>> 
>> The lazy list/map implementations are broken out from `ImmutableCollections` to a separate class.
>> 
>> The old benchmarks are not moved/renamed to allow comparison with previous releases.
>> 
>> `java.util.Optional` is updated so that its field is annotated with `@Stable`.  This is to allow `Optional` instances to be held in lazy constants and still provide constant folding.
>
> Per Minborg has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 112 commits:
> 
>  - Clarify toString spec
>  - Merge branch 'master' into lazy-constants
>  - Add @AOTSafeClassInitializer
>  - Address comments in PR
>  - Fix merge mistake
>  - Merge master
>  - Rework toString implementations
>  - Update after doc comments
>  - Merge branch 'master' into lazy-constants
>  - Revert the AbstractMap.keySet @Stable annotation
>  - ... and 102 more: https://git.openjdk.org/jdk/compare/76a1109d...1f439bec

Some initial comments on the spec and implementation. I still need to look at the tests.

src/java.base/share/classes/java/lang/LazyConstant.java line 48:

> 46:  * (provided at construction) will be invoked and the result will be used to initialize
> 47:  * the constant. Once a lazy constant is initialized, its contents can <em>never change</em>
> 48:  * and will be retrieved over and over again upon subsequent {@linkplain #get() get}

Are the above links to `#get()` also intended to have a the plain `get` text?

src/java.base/share/classes/java/lang/LazyConstant.java line 169:

> 167:  *          a lazy constant {@linkplain java.lang.ref##reachability strongly references}
> 168:  *          it contents. Hence, a lazy constant will hold its contents until
> 169:  *          the lazy constant itself is collected (if ever).

Maybe you could avoid the word 'collected' here, as there are no other references to the GC in this section, and no link to what GC is/does. i.e. I think 'collected' is not well-defined enough to be used here.

Suggestion:

 *          it contents. Hence, the contents of a lazy constant will be reachable as long
 *          as the lazy constant itself is reachable.

src/java.base/share/classes/java/lang/LazyConstant.java line 249:

> 247: 
> 248:     /**
> 249:      * {@return the {@linkplain System#identityHashCode(Object)} for this lazy constant}

How does this link render? It doesn't have any plain text. Maybe it's missing?

Suggestion:

     * {@return the {@linkplain System#identityHashCode(Object) identity hash code} for this lazy constant}

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

> 255: 
> 256:     /**
> 257:      * {@return a non-initializing string suitable for debugging}

What is a 'non-initializing string'? ;) I think the second paragraph already covers this aspect, so you can leave it out here.

src/java.base/share/classes/java/lang/LazyConstant.java line 275:

> 273:     /**
> 274:      * {@return a lazy constant to be computed later via the provided
> 275:      *          {@code computingFunction}}

The function computes the contents, not the LC itself, so:

Suggestion:

     * {@return a lazy constant whose contents is to be computed later via the provided
     *          {@code computingFunction}}

src/java.base/share/classes/java/util/LazyCollections.java line 55:

> 53: 
> 54:     // Unsafe allows LazyCollection classes to be used early in the boot sequence
> 55:     static final Unsafe UNSAFE = Unsafe.getUnsafe();

Suggestion:

    private static final Unsafe UNSAFE = Unsafe.getUnsafe();

src/java.base/share/classes/java/util/LazyCollections.java line 66:

> 64:         // using `elements.length`.
> 65:         @Stable
> 66:         private final int size;

Is this really that important? What about the extra footprint added by the `int` field? We might have many instances of this class, but only one copy of the bytecode.

src/java.base/share/classes/java/util/LazyCollections.java line 199:

> 197:                 final int size = base.size();
> 198:                 subListRangeCheck(fromIndex, toIndex, size);
> 199:                 return new ReverseOrderLazyListView<>(base.subList(size - toIndex, size - fromIndex));

Why not this? (maybe add a comment?)
Suggestion:

                return new ReverseOrderLazyListView<>(base.subList(fromIndex, toIndex));

src/java.base/share/classes/java/util/LazyCollections.java line 264:

> 262: 
> 263:         @Stable
> 264:         private final Map<K, Integer> indexMapper;

This index mapper is a clever solution that avoids implementing a hashing function again. Lookups through this map can be folded because it is created using `Map.ofEntries`. I think you should put that in a comment here as well.

src/java.base/share/classes/java/util/LazyCollections.java line 433:

> 431:             @Override public V      getValue() {
> 432:                 final int index = map.indexFor(getKey);
> 433:                 final V v = map.getAcquire(getKey);

I suppose you could use `orElseCompute` here, since you already have the index

src/java.base/share/classes/java/util/LazyCollections.java line 488:

> 486:     static final class Mutexes {
> 487: 
> 488:         static final Object TOMB_STONE = new Object();

Suggestion:

        private static final Object TOMB_STONE = new Object();

src/java.base/share/classes/java/util/LazyCollections.java line 578:

> 576:             if (t == null) {
> 577:                 final T newValue = switch (functionHolder.function()) {
> 578:                     case Supplier<?> sup     -> (T) sup.get();

Is the held function ever a Supplier? I don't see a FunctionHolder being created with one.

src/java.base/share/classes/java/util/LazyCollections.java line 607:

> 605:         assert Thread.holdsLock(mutex) : index + "didn't hold " + mutex;
> 606:         // We know we hold the monitor here so plain semantic is enough
> 607:         if (array[index] == null) {

This should always be true when we get here, right?

src/java.base/share/classes/java/util/List.java line 1222:

> 1220:      * <p>
> 1221:      * Any {@link List#subList(int, int) subList()} or {@link List#reversed()} views
> 1222:      * of the returned list are also lazily computed.

Is this really about the views itself? Or about the elements? (AFAIK these views are typically lazily computed/created for others List impls as well)

Suggestion:

     * The elements of any {@link List#subList(int, int) subList()} or {@link List#reversed()} views
     * of the returned list are also lazily computed.

src/java.base/share/classes/java/util/Map.java line 1765:

> 1763:      * at most once per key, even in a multi-threaded environment. Competing
> 1764:      * threads accessing a value already under computation will block until an element
> 1765:      * is computed or an exception is thrown by the computing thread.

I think technically it's more correct to say something like '... or the computing function completes abnormally'. Since, if an exception is throw inside the computing function (by the computing thread), it may be caught and handled inside the computing function as well.

src/java.base/share/classes/java/util/Map.java line 1778:

> 1776:      * <p>
> 1777:      * Any {@link Map#values()} or {@link Map#entrySet()} views of the returned map are
> 1778:      * also lazily computed.

Suggestion:

     * The values of any {@link Map#values()} or {@link Map#entrySet()} views of the returned map are
     * also lazily computed.

src/java.base/share/classes/java/util/Map.java line 1817:

> 1815:         final Set<K> keyCopies = Set.copyOf(keys);
> 1816:         Objects.requireNonNull(computingFunction);
> 1817:         if (keys instanceof EnumSet<?> && !keys.isEmpty()) {

The fact that `keys` is being used here and not `keyCopies` looks a bit fishy. In general we should use the validated copy of our data in subsequent processing. Since `EnumSet` is mutable, it seems that these two could go out of sync.

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

PR Review: https://git.openjdk.org/jdk/pull/27605#pullrequestreview-3448944313
PR Review Comment: https://git.openjdk.org/jdk/pull/27605#discussion_r2518699095
PR Review Comment: https://git.openjdk.org/jdk/pull/27605#discussion_r2518745565
PR Review Comment: https://git.openjdk.org/jdk/pull/27605#discussion_r2518765422
PR Review Comment: https://git.openjdk.org/jdk/pull/27605#discussion_r2518768972
PR Review Comment: https://git.openjdk.org/jdk/pull/27605#discussion_r2518779499
PR Review Comment: https://git.openjdk.org/jdk/pull/27605#discussion_r2518885149
PR Review Comment: https://git.openjdk.org/jdk/pull/27605#discussion_r2518897461
PR Review Comment: https://git.openjdk.org/jdk/pull/27605#discussion_r2518919343
PR Review Comment: https://git.openjdk.org/jdk/pull/27605#discussion_r2518968294
PR Review Comment: https://git.openjdk.org/jdk/pull/27605#discussion_r2518950708
PR Review Comment: https://git.openjdk.org/jdk/pull/27605#discussion_r2518970767
PR Review Comment: https://git.openjdk.org/jdk/pull/27605#discussion_r2519007486
PR Review Comment: https://git.openjdk.org/jdk/pull/27605#discussion_r2519002745
PR Review Comment: https://git.openjdk.org/jdk/pull/27605#discussion_r2518813240
PR Review Comment: https://git.openjdk.org/jdk/pull/27605#discussion_r2518832566
PR Review Comment: https://git.openjdk.org/jdk/pull/27605#discussion_r2518837814
PR Review Comment: https://git.openjdk.org/jdk/pull/27605#discussion_r2518880555

More information about the core-libs-dev mailing list