On Thu, 24 Apr 2025 10:37:59 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:
>
> Make public constuctor private
Looks mostly good. Left a few more inline comments
src/java.base/share/classes/java/lang/StableValue.java line 618:
> 616: *
> 617: * @param size the size of the allowed inputs in the continuous
> 618: * interval {@code [0, size)}
"Size of allowed inputs" doesn't seem quite right, maybe:
Suggestion:
* @param size the upper bound of the range {@code [0, size)}
indicating the allowed inputs
src/java.base/share/classes/java/lang/StableValue.java line 619:
> 617: * @param size the size of the allowed inputs in the continuous
> 618: * interval {@code [0, size)}
> 619: * @param underlying IntFunction used to compute cached values
Suggestion:
* @param underlying {@code IntFunction} used to compute cached values
test/jdk/java/lang/StableValue/StableFunctionTest.java line 79:
> 77: void factoryInvariants(Set<Value> inputs) {
> 78: assertThrows(NullPointerException.class, () ->
> StableValue.function(null, MAPPER));
> 79: assertThrows(NullPointerException.class, () ->
> StableValue.function(inputs, null));
I think you also need a case for `null` elements in `inputs`
test/jdk/java/lang/StableValue/StableValueTest.java line 373:
> 371: .toList();
> 372: threads.forEach(Thread::start);
> 373: LockSupport.parkNanos(TimeUnit.MILLISECONDS.toNanos(1));
Instead of parking and calling `countDown` here, which might be unreliable if a
particular thread doesn't get scheduled, you could set the latch value to
`noThreads`, and then do a `starter.countDown()` before calling `await`. That
should start all the threads running at the same time once all of them are
ready.
test/micro/org/openjdk/bench/java/lang/stable/StableMethodHandleBenchmark.java
line 121:
> 119: cp.nameAndTypeEntry(DEFAULT_NAME, CD_MethodHandle)));
> 120: return null;
> 121: }
Seems unused
-------------
PR Review: https://git.openjdk.org/jdk/pull/23972#pullrequestreview-2803046751
PR Review Comment: https://git.openjdk.org/jdk/pull/23972#discussion_r2066109981
PR Review Comment: https://git.openjdk.org/jdk/pull/23972#discussion_r2066113200
PR Review Comment: https://git.openjdk.org/jdk/pull/23972#discussion_r2066747882
PR Review Comment: https://git.openjdk.org/jdk/pull/23972#discussion_r2066781044
PR Review Comment: https://git.openjdk.org/jdk/pull/23972#discussion_r2066809541
