RFR: 8330465: Stable Values and Collections (Internal)

Dan Heidinga heidinga at openjdk.org
Tue May 14 14:14:24 UTC 2024


On Tue, 16 Apr 2024 11:47:23 GMT, Per Minborg <pminborg at 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...

src/java.base/share/classes/java/lang/reflect/AccessibleObject.java line 193:

> 191:      * <li>final fields declared in a {@linkplain Class#isHidden() hidden class}</li>
> 192:      * <li>final fields declared in a {@linkplain Class#isRecord() record}</li>
> 193:      * <li>final fields of type {@linkplain StableValue StableValue}</li>

In Valhalla, we've been looking at adding "strict" final fields to support value classes (which must be strongly immutable) which are fields that are unmodifiable.  Most of the existing unmodifiable field cases can be covered by "strict" fields.  This one can't though so I'm a little saddened to see this list grow.

src/java.base/share/classes/java/lang/reflect/Field.java line 179:

> 177:         AccessibleObject.checkPermission();
> 178:         if (flag) {
> 179:             if (StableValue.class.isAssignableFrom(type) && Modifier.isFinal(modifiers)) {

Should this check be done regardless of the value of "flag"?  If it failed always when calling ::setAccessible on a StableValue, we'd make it easier to find bugs and the contract for users would be clearer

src/java.base/share/classes/java/lang/reflect/Field.java line 181:

> 179:             if (StableValue.class.isAssignableFrom(type) && Modifier.isFinal(modifiers)) {
> 180:                 throw newInaccessibleObjectException(
> 181:                         "Unable to make field " + this + " accessable: " +

Suggestion:

                        "Unable to make field " + this + " accessible: " +

src/java.base/share/classes/java/util/ImmutableCollections.java line 183:

> 181:                                                K key,
> 182:                                                Function<? super K, ? extends V> mapper) {
> 183:                     if (map instanceof HasComputeIfUnset) {

Can we use pattern matching instanceof here?

if (map instance HasComputeIfUnset uc) {

src/java.base/share/classes/jdk/internal/lang/StableArray.java line 25:

> 23:  * @since 23
> 24:  */
> 25: public sealed interface StableArray<V>

Do we have a use case for StableArray beyond those of StableList?

src/java.base/share/classes/jdk/internal/lang/StableValue.java line 47:

> 45:  * An atomic, thread-safe, stable value holder for which the value can be set at most once.
> 46:  * <p>
> 47:  * Stable values are eligible for constant folding and other optimizations by the JVM.

Other values are also eligible for constant folding.  Trying to spec in terms of the optimizations that the JVM may do is usually an unstable state.  Better to spec in terms of what the user observable behaviour is and leave it at something like "unlocks further JVM optimizations".

src/java.base/share/classes/jdk/internal/lang/StableValue.java line 130:

> 128:      * } else {
> 129:      *     V newValue = supplier.get();
> 130:      *     stable.setOrThrow(newValue);

If ::computeIfUnset allows racy sets, then it isn't equivalent to this code as ::setOrThrow will throw on a race, correct?

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/18794#discussion_r1575048707
PR Review Comment: https://git.openjdk.org/jdk/pull/18794#discussion_r1575005661
PR Review Comment: https://git.openjdk.org/jdk/pull/18794#discussion_r1574997174
PR Review Comment: https://git.openjdk.org/jdk/pull/18794#discussion_r1575014234
PR Review Comment: https://git.openjdk.org/jdk/pull/18794#discussion_r1584931870
PR Review Comment: https://git.openjdk.org/jdk/pull/18794#discussion_r1575025174
PR Review Comment: https://git.openjdk.org/jdk/pull/18794#discussion_r1575030995


More information about the hotspot-compiler-dev mailing list