On Mon, 17 Nov 2025 11:38:03 GMT, Per Minborg <[email protected]> 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 incrementally with one additional 
> commit since the last revision:
> 
>   Fix typo

I've had a look mostly at the docs, everything I'm saying is optional (though 
I'd really like to see the Logger example changed).

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

> 40:  * A lazy constant is created using the factory method
> 41:  * {@linkplain LazyConstant#of(Supplier) LazyConstant.of({@code 
> <computing function>})}.
> 42:  * When created, the lazy constant is <em>not initialized</em>, meaning 
> it has no contents.

Subjectively, might this be clearer as a bulleted list? This is a sequence of 
statements about the state transitions after all.

<ul>
<li>On creation the lazy constant is <em>not initialized</em> ...
<li>The first time {@linkplain #get() get()} is called ...
<li>Once a lazy constant is initialized ...

Or just splitting by paragraph.

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

> 58:  *    private final LazyConstant<Logger> logger =
> 59:  *            // @link substring="of" target="#of" :
> 60:  *            LazyConstant.of( () -> Logger.create(Component.class) );

We had previously talked about whether the `Logger` example is a good one.

Personally I think it's a bad example if it encourages people to use loggers 
this way since:
* non static loggers aren't suitable to logging in static methods.
* The Logger framework has a perfectly functional mechanism for injecting 
static logger instances.
* Over-use of non-static logger instances pollutes internal APIs and ends up 
leaking a dependency on a specific logger instance, making refactoring hard.

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

> 64:  *        // ...
> 65:  *    }
> 66:  * }

Arguably weird/wrong indents on closing '}' ?

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

> 239: 
> 240:     /**
> 241:      * {@return if this lazy constant is the same as the provided {@code 
> obj}}

Maybe say "is the same instance as ...", then you might be able to shorten the 
"In other words" paragraph below as they sort of say the same thing in almost 
the same words.

As a reader I'm more interested in *why* this slightly unexpected behaviour 
happens. Maybe a section in the class level docs, linked from here, explaining 
the issues with using the content for equals/hashCode would be better (that can 
also talk about lazy constants being mostly unsuitable as key types).

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

> 281:      * <p>
> 282:      * The returned lazy constant strongly references the provided
> 283:      * {@code computingFunction} at least until initialization completes 
> successfully.

As a reader I'm a bit nervous that I don't know if it will ever drop the 
function. Suppose my init relies on something chunky I want to be sure has gone 
away?

src/java.base/share/classes/java/nio/charset/Charset.java line 1:

> 1: /*

Side note: I'm slightly surprised to see the introduction and the usage 
happening in the same PR. Is this really being submitting in one go?

src/java.base/share/classes/java/nio/charset/Charset.java line 619:

> 617: 
> 618:     private static final LazyConstant<Charset> defaultCharset = 
> LazyConstant.of(
> 619:             new Supplier<>() { public Charset get() { return 
> defaultCharset0(); }});

Can these not be done with lambdas?

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

> 1208:      * invoked at most once per list index, even in a multi-threaded 
> environment.
> 1209:      * Competing threads accessing an element already under computation 
> will block until
> 1210:      * an element is computed or the computing function completes 
> abnormally

Missing full-stop.

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

> 1236:      * <p>
> 1237:      * The returned lazy list strongly references its computing
> 1238:      * function used to compute elements at least so long as there are 
> uninitialized

"at least so long as" sounds weird to me.

I think you would say either:
* "... at least *as* long as there are uninitialized elmements."
* "... so long as there are uninitialized elmements."

But you could always say:
* "... while there are uninitialized elements"

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

PR Review: https://git.openjdk.org/jdk/pull/27605#pullrequestreview-3472336926
PR Review Comment: https://git.openjdk.org/jdk/pull/27605#discussion_r2533786705
PR Review Comment: https://git.openjdk.org/jdk/pull/27605#discussion_r2533801082
PR Review Comment: https://git.openjdk.org/jdk/pull/27605#discussion_r2533773546
PR Review Comment: https://git.openjdk.org/jdk/pull/27605#discussion_r2533830776
PR Review Comment: https://git.openjdk.org/jdk/pull/27605#discussion_r2533836397
PR Review Comment: https://git.openjdk.org/jdk/pull/27605#discussion_r2533841573
PR Review Comment: https://git.openjdk.org/jdk/pull/27605#discussion_r2533846906
PR Review Comment: https://git.openjdk.org/jdk/pull/27605#discussion_r2533855814
PR Review Comment: https://git.openjdk.org/jdk/pull/27605#discussion_r2533865784

Reply via email to