RFR: 8330465: Stable Values and Collections (Internal) [v5]

[Per Minborg]

On Wed, 15 May 2024 18:45:16 GMT, ExE Boss <duke at openjdk.org> wrote:

>> src/java.base/share/classes/jdk/internal/lang/stable/StableValueImpl.java line 104:
>> 
>>> 102:         // Optimistically try plain semantics first
>>> 103:         final V v = value;
>>> 104:         if (v != null) {
>> 
>> If `value == null && state == NULL`, can the path still be constant folded? I doubt it because `value` in this case may not be promoted to constant.
>
> Maybe the `state == NULL` check should be moved before `v != null`, as the **JIT** doesn’t constant‑fold `null` [`@Stable`] values:
> https://github.com/openjdk/jdk/blob/8a4315f833f3700075d65fae6bc566011c837c07/src/java.base/share/classes/jdk/internal/vm/annotation/Stable.java#L41-L44  https://github.com/openjdk/jdk/blob/8a4315f833f3700075d65fae6bc566011c837c07/src/java.base/share/classes/jdk/internal/vm/annotation/Stable.java#L64-L71
> 
> [`@Stable`]: https://github.com/openjdk/jdk/blob/master/src/java.base/share/classes/jdk/internal/vm/annotation/Stable.java

It seems reasonable to assume `null` values are not constant-folded. For straight-out-of-the-box usage, there is no apparent significant difference as indicated by a new benchmark I just added:


Benchmark                                  Mode  Cnt      Score      Error   Units
StableStaticBenchmark.atomic              thrpt   10   5729.683 ?  502.023  ops/us
StableStaticBenchmark.dcl                 thrpt   10   6069.222 ?  951.784  ops/us
StableStaticBenchmark.dclHolder           thrpt   10   5502.102 ? 1630.627  ops/us
StableStaticBenchmark.stable              thrpt   10  12737.158 ? 1746.456  ops/us <- Non-null benchmark
StableStaticBenchmark.stableHolder        thrpt   10  12053.978 ? 1421.527  ops/us
StableStaticBenchmark.stableList          thrpt   10  12443.870 ? 2084.607  ops/us
StableStaticBenchmark.stableNull          thrpt   10  13164.232 ?  591.284  ops/us <- Added null benchmark
StableStaticBenchmark.stableRecordHolder  thrpt   10  13638.893 ? 1250.895  ops/us
StableStaticBenchmark.staticCHI           thrpt   10  13639.220 ? 1190.922  ops/us


If the `null` value participates in a much larger constant-folding tree, there might be a significant difference. I was afraid moving the order would have detrimental effects on instance performance but that does not seem to be the case:

Checking value first:


Benchmark                           Mode  Cnt     Score      Error   Units
StableBenchmark.atomic             thrpt   10   246.460 ?   75.417  ops/us
StableBenchmark.dcl                thrpt   10   243.481 ?   35.021  ops/us
StableBenchmark.stable             thrpt   10  4977.693 ?  675.926  ops/us  <- Non-null
StableBenchmark.stableHoldingList  thrpt   10  3614.460 ?  275.140  ops/us
StableBenchmark.stableList         thrpt   10  3328.155 ?  898.202  ops/us
StableBenchmark.stableListStored   thrpt   10  3842.174 ?  535.902  ops/us
StableBenchmark.stableNull         thrpt   10  6217.737 ?  840.376  ops/us <- null
StableBenchmark.supplier           thrpt   10  9369.934 ? 1449.182  ops/us


Checking null first:


Benchmark                           Mode  Cnt     Score      Error   Units
StableBenchmark.atomic             thrpt   10   275.952 ?   39.480  ops/us
StableBenchmark.dcl                thrpt   10   252.697 ?   18.645  ops/us
StableBenchmark.stable             thrpt   10  5211.552 ?  315.307  ops/us <- Non-null
StableBenchmark.stableHoldingList  thrpt   10  3764.202 ?  224.325  ops/us
StableBenchmark.stableList         thrpt   10  3689.870 ?  419.858  ops/us
StableBenchmark.stableListStored   thrpt   10  3676.182 ?  938.485  ops/us
StableBenchmark.stableNull         thrpt   10  6046.935 ? 1512.391  ops/us <- null
StableBenchmark.supplier           thrpt   10  9202.202 ? 1479.950  ops/us


So, swapping order seems to be the right move.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/18794#discussion_r1602719002