On Mon, 10 Mar 2025 18:11:23 GMT, Per Minborg <pminb...@openjdk.org> wrote:
> Implement JEP 502.
>
> The PR passes tier1-tier3 tests.
src/java.base/share/classes/java/lang/StableValue.java line 79:
> 77: * logger.trySet(Logger.create(Component.class));
> 78: * }
> 79: * return logger.orThrow();
Suggestion:
* return logger.orElseThrow();
src/java.base/share/classes/java/lang/StableValue.java line 127:
> 125: * evaluated only once, even when {@code logger.orElseSet()} is invoked
> concurrently.
> 126: * This property is crucial as evaluation of the supplier may have side
> effects,
> 127: * e.g., the call above to {@code Logger.getLogger()} may result in
> storage resources
Suggestion:
* e.g., the call above to {@code Logger.create()} may result in storage
resources
src/java.base/share/classes/java/lang/StableValue.java line 344:
> 342: * {@linkplain java.lang.ref##reachability reachable} stable values will
> hold their set
> 343: * content perpetually.
> 344: * <p>
Should the original functions / mappers (for stable functions and collections)
also stay reachable? Kotlin's
[`Lazy`](https://kotlinlang.org/api/core/kotlin-stdlib/kotlin/-lazy/) [nulls
out](https://github.com/JetBrains/kotlin/blob/c6f337283d59fcede75954eebaa589ad1b479aea/libraries/stdlib/jvm/src/kotlin/util/LazyJVM.kt#L70-L89)
the initializer function when it's no longer needed.
src/java.base/share/classes/java/lang/StableValue.java line 423:
> 421: * {@snippet lang=java:
> 422: * if (stable.isSet()) {
> 423: * return stable.get();
Suggestion:
* return stable.orElseThrow();
src/java.base/share/classes/java/lang/StableValue.java line 547:
> 545: IntFunction<? extends R>
> original) {
> 546: if (size < 0) {
> 547: throw new IllegalArgumentException();
This exceptions isn't documented, same for `StableValue.list()`
src/java.base/share/classes/jdk/internal/lang/stable/StableEnumFunction.java
line 112:
> 110: final Class<E> enumType =
> (Class<E>)inputs.iterator().next().getClass();
> 111: return (Function<T, R>) new StableEnumFunction<E, R>(enumType,
> min, StableValueFactories.array(size), (Function<E, R>) original);
> 112: }
If `inputs` contains the enumuration constants with ordinals 0 and 2, wouldn't
this code wrongly cause the enumeration constant with ordinal 1 to be an
allowed input?
src/java.base/share/classes/jdk/internal/lang/stable/StableValueImpl.java line
141:
> 139: ? "(this StableValue)"
> 140: : "StableValue" + renderWrapped(t);
> 141: }
Are deeper cycles of concern? I was thinking of this:
var a = StableValue.of();
var b = StableValue.of();
a.trySet(b);
b.trySet(a);
System.out.println(a);
This would solve deeper cycles for `StableValueImpl`:
@Override
public String toString() {
final StringBuilder sb = new StringBuilder("StableValue");
int depth = 0;
Object t = value;
while (t instanceof StableValueImpl<?> s) {
if (s == this) {
t = "(this StableValue)";
break;
}
sb.append("[StableValue");
depth++;
t = s.value;
}
sb.append(renderWrapped(t));
while (depth-- > 0) sb.append(']');
return sb.toString();
}
This might also apply to stable functions and collections, I haven't thought it
through for them.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/23972#discussion_r1989143787
PR Review Comment: https://git.openjdk.org/jdk/pull/23972#discussion_r1989165612
PR Review Comment: https://git.openjdk.org/jdk/pull/23972#discussion_r1989265489
PR Review Comment: https://git.openjdk.org/jdk/pull/23972#discussion_r1989377859
PR Review Comment: https://git.openjdk.org/jdk/pull/23972#discussion_r1989504117
PR Review Comment: https://git.openjdk.org/jdk/pull/23972#discussion_r1988064795
PR Review Comment: https://git.openjdk.org/jdk/pull/23972#discussion_r1988092230
