RFR: 8351565: Implement JEP 502: Stable Values (Preview) [v78]

Jorn Vernee jvernee at openjdk.org
Tue Apr 29 15:29:19 UTC 2025


On Thu, 24 Apr 2025 10:37:59 GMT, Per Minborg <pminborg at 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


More information about the core-libs-dev mailing list