On Tue, 16 Apr 2024 11:47:23 GMT, Per Minborg <pminb...@openjdk.org> wrote:
> # Stable Values & Collections (Internal) > > ## Summary > This PR proposes to introduce an internal _Stable Values & Collections_ API, > which provides immutable value holders where elements are initialized _at > most once_. Stable Values & Collections offer the performance and safety > benefits of final fields while offering greater flexibility as to the timing > of initialization. > > ## Goals > * Provide an easy and intuitive API to describe value holders that can > change at most once. > * Decouple declaration from initialization without significant footprint or > performance penalties. > * Reduce the amount of static initializer and/or field initialization code. > * Uphold integrity and consistency, even in a multi-threaded environment. > > For more details, see the draft JEP: https://openjdk.org/jeps/8312611 > > ## Performance > Performance compared to instance variables using an `AtomicReference` and one > protected by double-checked locking under concurrent access by 8 threads: > > > Benchmark Mode Cnt Score Error Units > StableBenchmark.instanceAtomic avgt 10 1.576 ? 0.052 ns/op > StableBenchmark.instanceDCL avgt 10 1.608 ? 0.059 ns/op > StableBenchmark.instanceStable avgt 10 0.979 ? 0.023 ns/op <- > StableValue (~40% faster than DCL) > > > Performance compared to static variables protected by `AtomicReference`, > class-holder idiom holder, and double-checked locking (8 threads): > > > Benchmark Mode Cnt Score Error Units > StableBenchmark.staticAtomic avgt 10 1.335 ? 0.056 ns/op > StableBenchmark.staticCHI avgt 10 0.623 ? 0.086 ns/op > StableBenchmark.staticDCL avgt 10 1.418 ? 0.171 ns/op > StableBenchmark.staticList avgt 10 0.617 ? 0.024 ns/op > StableBenchmark.staticStable avgt 10 0.604 ? 0.022 ns/op <- > StableValue ( > 2x faster than `AtomicInteger` and DCL) > > > Performance for stable lists in both instance and static contexts whereby the > sum of random contents is calculated for stable lists (which are thread-safe) > compared to `ArrayList` instances (which are not thread-safe) (under single > thread access): > > > Benchmark Mode Cnt Score Error Units > StableListSumBenchmark.instanceArrayList avgt 10 0.356 ? 0.005 ns/op > StableListSumBenchmark.instanceList avgt 10 0.373 ? 0.017 ns/op > <- Stable list > StableListSumBenchmark.staticArrayList avgt 10 0.352 ? 0.002 ns/op > StableListSumBenchmark.staticList avgt 10 0.356 ? 0.00... **Nit:** Inconsistent whitespace: src/java.base/share/classes/java/lang/reflect/AccessibleObject.java line 393: > 391: } > 392: > 393: InaccessibleObjectException newInaccessibleObjectException(String > msg) { This internal helper method can be `static`: Suggestion: static InaccessibleObjectException newInaccessibleObjectException(String msg) { src/java.base/share/classes/jdk/internal/lang/stable/StableValueElement.java line 57: > 55: return switch (stateVolatile()) { > 56: case UNSET -> throw new NoSuchElementException(); // No > value was set > 57: case NON_NULL -> orThrowVolatile(); // Race: another thread > has set a value It should be safe to avoid self‑recursion here: Suggestion: case NON_NULL -> elements[index]; // Race: another thread has set a value or: Suggestion: case NON_NULL -> { v = elements[index]; // Race: another thread has set a value if (v != null) { yield v; } throw shouldNotReachHere(); } src/java.base/share/classes/jdk/internal/lang/stable/StableValueElement.java line 63: > 61: // more compact byte code. > 62: switch (stateVolatile()) { > 63: case UNSET: { throw StableUtil.notSet();} Suggestion: case UNSET: { throw StableUtil.notSet(); } src/java.base/share/classes/jdk/internal/lang/stable/StableValueElement.java line 116: > 114: public V computeIfUnset(Supplier<? extends V> supplier) { > 115: // Todo: This creates a lambda > 116: return computeIfUnsetShared(supplier, Supplier::get); Suggestion: return computeIfUnsetShared(supplier, supplierExtractor()); src/java.base/share/classes/jdk/internal/lang/stable/StableValueElement.java line 144: > 142: // more compact byte code. > 143: switch (stateVolatile()) { > 144: case UNSET: { return computeIfUnsetVolatile0(provider, > key);} Suggestion: case UNSET: { return computeIfUnsetVolatile0(provider, key); } src/java.base/share/classes/jdk/internal/lang/stable/StableValueImpl.java line 116: > 114: // more compact byte code. > 115: switch (stateVolatile()) { > 116: case UNSET: { throw StableUtil.notSet();} Suggestion: case UNSET: { throw StableUtil.notSet(); } src/java.base/share/classes/jdk/internal/lang/stable/StableValueImpl.java line 181: > 179: // more compact byte code. > 180: switch (stateVolatile()) { > 181: case UNSET: { return computeIfUnsetVolatile0(supplier);} Suggestion: case UNSET: { return computeIfUnsetVolatile0(supplier); } ------------- PR Review: https://git.openjdk.org/jdk/pull/18794#pullrequestreview-2029063641 PR Review Comment: https://git.openjdk.org/jdk/pull/18794#discussion_r1570620018 PR Review Comment: https://git.openjdk.org/jdk/pull/18794#discussion_r1580882515 PR Review Comment: https://git.openjdk.org/jdk/pull/18794#discussion_r1583418107 PR Review Comment: https://git.openjdk.org/jdk/pull/18794#discussion_r1572101643 PR Review Comment: https://git.openjdk.org/jdk/pull/18794#discussion_r1583418492 PR Review Comment: https://git.openjdk.org/jdk/pull/18794#discussion_r1583419719 PR Review Comment: https://git.openjdk.org/jdk/pull/18794#discussion_r1583420000
