On Thu, 10 Apr 2025 10:26:36 GMT, Per Minborg <pminb...@openjdk.org> wrote:
>> Implement JEP 502.
>>
>> The PR passes tier1-tier3 tests.
>
> Per Minborg has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Improve docs as per comments
src/java.base/share/classes/java/lang/StableValue.java line 93:
> 91: * <p>
> 92: * In order to guarantee that, even under races, only one instance of
> {@code Logger} is
> 93: * evee created, the {@linkplain #orElseSet(Supplier) orElseSet()} method
> can be used
Typo:
Suggestion:
* ever created, the {@linkplain #orElseSet(Supplier) orElseSet()} method can
be used
src/java.base/share/classes/java/lang/StableValue.java line 94:
> 92: * In order to guarantee that, even under races, only one instance of
> {@code Logger} is
> 93: * evee created, the {@linkplain #orElseSet(Supplier) orElseSet()} method
> can be used
> 94: * instead, where the content is atomically and lazily computed via a
Suggestion:
* instead, where the content is lazily computed, and atomically set, via a
src/java.base/share/classes/java/lang/StableValue.java line 120:
> 118: * evaluates and sets the content; the content is then returned to the
> client. In other
> 119: * words, {@code orElseSet()} guarantees that a stable value's content
> is <em>set</em>
> 120: * before it is used.
Suggestion:
* before it is returned.
src/java.base/share/classes/java/lang/StableValue.java line 131:
> 129: * Stable values provide the foundation for higher-level functional
> abstractions. A
> 130: * <em>stable supplier</em> is a supplier that computes a value and then
> caches it into
> 131: * a backing stable value storage for later use. A stable supplier is
> created via the
Suggestion:
* a backing stable value storage for subsequent use. A stable supplier is
created via the
src/java.base/share/classes/java/lang/StableValue.java line 133:
> 131: * a backing stable value storage for later use. A stable supplier is
> created via the
> 132: * {@linkplain StableValue#supplier(Supplier) StableValue.supplier()}
> factory, by
> 133: * providing an original {@linkplain Supplier} which is invoked when the
> stable supplier
Suggestion:
* providing an underlying {@linkplain Supplier} which is invoked when the
stable supplier
src/java.base/share/classes/java/lang/StableValue.java line 169:
> 167: * private static final int SIZE = 6;
> 168: * private static final IntFunction<Integer> ORIGINAL_POWER_OF_TWO =
> 169: * v -> 1 << v;
Suggestion:
* v -> 1 << v;
src/java.base/share/classes/java/lang/StableValue.java line 173:
> 171: * private static final IntFunction<Integer> POWER_OF_TWO =
> 172: * // @link substring="intFunction"
> target="#intFunction(int,IntFunction)" :
> 173: * StableValue.intFunction(SIZE, ORIGINAL_POWER_OF_TWO);
Suggestion:
* // @link substring="intFunction"
target="#intFunction(int,IntFunction)" :
* StableValue.intFunction(SIZE, ORIGINAL_POWER_OF_TWO);
src/java.base/share/classes/java/lang/StableValue.java line 180:
> 178: * }
> 179: *
> 180: * int pwr4 = PowerOf2Util.powerOfTwo(4); // May eventually constant
> fold to 16 at runtime
Suggestion:
* int result = PowerOf2Util.powerOfTwo(4); // May eventually constant fold
to 16 at runtime
src/java.base/share/classes/java/lang/StableValue.java line 208:
> 206: * private static final Function<Integer, Integer> LOG2 =
> 207: * // @link substring="function"
> target="#function(Set,Function)" :
> 208: * StableValue.function(KEYS, ORIGINAL_LOG2);
Suggestion:
* private static final Set<Integer> KEYS =
* Set.of(1, 2, 4, 8, 16, 32);
* private static final UnaryOperator<Integer> ORIGINAL_LOG2 =
* i -> 31 - Integer.numberOfLeadingZeros(i);
*
* private static final Function<Integer, Integer> LOG2 =
* // @link substring="function" target="#function(Set,Function)" :
* StableValue.function(KEYS, ORIGINAL_LOG2);
src/java.base/share/classes/java/lang/StableValue.java line 216:
> 214: * }
> 215: *
> 216: * int log16 = Log2Util.log2(16); // May eventually constant fold to 4
> at runtime
Suggestion:
* int result = Log2Util.log2(16); // May eventually constant fold to 4 at
runtime
src/java.base/share/classes/java/lang/StableValue.java line 240:
> 238: * private static final List<Integer> POWER_OF_TWO =
> 239: * // @link substring="list" target="#list(int,IntFunction)"
> :
> 240: * StableValue.list(SIZE, ORIGINAL_POWER_OF_TWO);
Suggestion:
* v -> 1 << v;
*
* private static final List<Integer> POWER_OF_TWO =
* // @link substring="list" target="#list(int,IntFunction)" :
* StableValue.list(SIZE, ORIGINAL_POWER_OF_TWO);
src/java.base/share/classes/java/lang/StableValue.java line 247:
> 245: * }
> 246: *
> 247: * int pwr4 = PowerOf2Util.powerOfTwo(4); // May eventually constant
> fold to 16 at runtime
Suggestion:
* int result = PowerOf2Util.powerOfTwo(4); // May eventually constant fold
to 16 at runtime
src/java.base/share/classes/java/lang/StableValue.java line 267:
> 265: * private static final Map<Integer, INTEGER> LOG2 =
> 266: * // @link substring="map" target="#map(Set,Function)" :
> 267: * StableValue.map(CACHED_KEYS, LOG2_ORIGINAL);
Suggestion:
* Set.of(1, 2, 4, 8, 16, 32);
* private static final UnaryOperator<Integer> LOG2_ORIGINAL =
* i -> 31 - Integer.numberOfLeadingZeros(i);
*
* private static final Map<Integer, INTEGER> LOG2 =
* // @link substring="map" target="#map(Set,Function)" :
* StableValue.map(CACHED_KEYS, LOG2_ORIGINAL);
src/java.base/share/classes/java/lang/StableValue.java line 275:
> 273: * }
> 274: *
> 275: * int log16 = Log2Util.log2(16); // May eventually constant fold to 4
> at runtime
Suggestion:
* int result = Log2Util.log2(16); // May eventually constant fold to 4 at
runtime
src/java.base/share/classes/java/lang/StableValue.java line 318:
> 316: * <p>
> 317: * Here is another example where a more complex dependency graph is
> created in which
> 318: * integers in the Fibonacci delta series are lazily computed:
I'd probably reword this to: "Another example, which has a more complex
dependency graph, is computing the Fibonacci sequence:"
src/java.base/share/classes/java/lang/StableValue.java line 327:
> 325: *
> 326: * private static final IntFunction<Integer> FIB =
> 327: * StableValue.intFunction(MAX_SIZE_INT, Fibonacci::fib);
Suggestion:
* StableValue.intFunction(MAX_SIZE_INT, Fibonacci::fib);
src/java.base/share/classes/java/lang/StableValue.java line 357:
> 355: *
> 356: * If there are circular dependencies in a dependency graph, a stable
> value will
> 357: * eventually throw an {@linkplain IllegalStateException} upon
> referencing elements in
Suggestion:
* eventually throw an {@linkplain IllegalStateException} upon detecting
src/java.base/share/classes/java/lang/StableValue.java line 491:
> 489: *
> 490: * {@snippet lang=java:
> 491: * Value witness = stable.orElseSet(Value::new);
Suggestion:
* Value v = stable.orElseSet(Value::new);
src/java.base/share/classes/java/lang/StableValue.java line 565:
> 563: * the returned supplier's {@linkplain Supplier#get() get()} method.
> 564: * <p>
> 565: * The provided {@code original} supplier is guaranteed to be
> successfully invoked
Personally I prefer "underlying" over "original".
src/java.base/share/classes/java/lang/StableValue.java line 582:
> 580: static <T> Supplier<T> supplier(Supplier<? extends T> original) {
> 581: Objects.requireNonNull(original);
> 582: return StableSupplier.of(original);
I guess if `original` is a StableSupplier then we could just return that?
src/java.base/share/classes/java/lang/StableValue.java line 614:
> 612: * @throws IllegalArgumentException if the provided {@code size} is
> negative.
> 613: */
> 614: static <R> IntFunction<R> intFunction(int size,
Does an intFunction of size 0 make sense?
test/jdk/java/lang/StableValue/StableFunctionTest.java line 25:
> 23:
> 24: /* @test
> 25: * @summary Basic tests for StableFunctionTest methods
Suggestion:
* @summary Basic tests for StableFunction methods
test/jdk/java/lang/StableValue/StableIntFunctionTest.java line 25:
> 23:
> 24: /* @test
> 25: * @summary Basic tests for StableIntFunctionTest methods
Suggestion:
* @summary Basic tests for StableIntFunction methods
test/jdk/java/lang/StableValue/StableListTest.java line 25:
> 23:
> 24: /* @test
> 25: * @summary Basic tests for LazyList methods
Suggestion:
* @summary Basic tests for StableList methods
test/jdk/java/lang/StableValue/StableMapTest.java line 25:
> 23:
> 24: /* @test
> 25: * @summary Basic tests for LazyMap methods
Suggestion:
* @summary Basic tests for StableMap methods
test/jdk/java/lang/StableValue/StableSupplierTest.java line 25:
> 23:
> 24: /* @test
> 25: * @summary Basic tests for StableSupplierTest methods
Suggestion:
* @summary Basic tests for StableSupplier methods
test/jdk/java/lang/StableValue/StableValueFactoriesTest.java line 25:
> 23:
> 24: /* @test
> 25: * @summary Basic tests for StableValueFactoriesTest implementations
Suggestion:
* @summary Basic tests for StableValue factory implementations
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/23972#discussion_r2037168882
PR Review Comment: https://git.openjdk.org/jdk/pull/23972#discussion_r2037169961
PR Review Comment: https://git.openjdk.org/jdk/pull/23972#discussion_r2037172243
PR Review Comment: https://git.openjdk.org/jdk/pull/23972#discussion_r2037174262
PR Review Comment: https://git.openjdk.org/jdk/pull/23972#discussion_r2037174898
PR Review Comment: https://git.openjdk.org/jdk/pull/23972#discussion_r2037177853
PR Review Comment: https://git.openjdk.org/jdk/pull/23972#discussion_r2037178275
PR Review Comment: https://git.openjdk.org/jdk/pull/23972#discussion_r2037179545
PR Review Comment: https://git.openjdk.org/jdk/pull/23972#discussion_r2037180933
PR Review Comment: https://git.openjdk.org/jdk/pull/23972#discussion_r2037181796
PR Review Comment: https://git.openjdk.org/jdk/pull/23972#discussion_r2037193456
PR Review Comment: https://git.openjdk.org/jdk/pull/23972#discussion_r2037194051
PR Review Comment: https://git.openjdk.org/jdk/pull/23972#discussion_r2037195527
PR Review Comment: https://git.openjdk.org/jdk/pull/23972#discussion_r2037195700
PR Review Comment: https://git.openjdk.org/jdk/pull/23972#discussion_r2037202304
PR Review Comment: https://git.openjdk.org/jdk/pull/23972#discussion_r2037229765
PR Review Comment: https://git.openjdk.org/jdk/pull/23972#discussion_r2037234988
PR Review Comment: https://git.openjdk.org/jdk/pull/23972#discussion_r2037250964
PR Review Comment: https://git.openjdk.org/jdk/pull/23972#discussion_r2037256596
PR Review Comment: https://git.openjdk.org/jdk/pull/23972#discussion_r2037258059
PR Review Comment: https://git.openjdk.org/jdk/pull/23972#discussion_r2037261762
PR Review Comment: https://git.openjdk.org/jdk/pull/23972#discussion_r2037279107
PR Review Comment: https://git.openjdk.org/jdk/pull/23972#discussion_r2037278760
PR Review Comment: https://git.openjdk.org/jdk/pull/23972#discussion_r2037278239
PR Review Comment: https://git.openjdk.org/jdk/pull/23972#discussion_r2037279632
PR Review Comment: https://git.openjdk.org/jdk/pull/23972#discussion_r2037280278
PR Review Comment: https://git.openjdk.org/jdk/pull/23972#discussion_r2037282580
