RFR: 8350118: Simplify the layout access VarHandle [v2]

[Maurizio Cimadamore]

On Tue, 25 Feb 2025 09:51:08 GMT, Maurizio Cimadamore <mcimadamore at openjdk.org> wrote:

>> Benchmark results for the latest revision appears performance neutral. bytestacks same as last revision. 11 jobs left in tier 1-3, no failure so far. Also created CSR for this minor behavioral change.
>> 
>> Benchmark                                                (polluteProfile)  Mode  Cnt  Score   Error  Units
>> LoopOverNonConstantHeap.BB_get                                      false  avgt   30  0.934 ± 0.027  ns/op
>> LoopOverNonConstantHeap.BB_get                                       true  avgt   30  0.946 ± 0.028  ns/op
>> LoopOverNonConstantHeap.BB_loop                                     false  avgt   30  0.208 ± 0.004  ms/op
>> LoopOverNonConstantHeap.BB_loop                                      true  avgt   30  0.211 ± 0.003  ms/op
>> LoopOverNonConstantHeap.segment_get                                 false  avgt   30  1.123 ± 0.040  ns/op
>> LoopOverNonConstantHeap.segment_get                                  true  avgt   30  1.120 ± 0.040  ns/op
>> LoopOverNonConstantHeap.segment_loop                                false  avgt   30  0.205 ± 0.004  ms/op
>> LoopOverNonConstantHeap.segment_loop                                 true  avgt   30  0.202 ± 0.003  ms/op
>> LoopOverNonConstantHeap.segment_loop_instance                       false  avgt   30  0.209 ± 0.005  ms/op
>> LoopOverNonConstantHeap.segment_loop_instance                        true  avgt   30  0.202 ± 0.003  ms/op
>> LoopOverNonConstantHeap.segment_loop_instance_unaligned             false  avgt   30  0.209 ± 0.004  ms/op
>> LoopOverNonConstantHeap.segment_loop_instance_unaligned              true  avgt   30  0.210 ± 0.004  ms/op
>> LoopOverNonConstantHeap.segment_loop_readonly                       false  avgt   30  0.206 ± 0.004  ms/op
>> LoopOverNonConstantHeap.segment_loop_readonly                        true  avgt   30  0.206 ± 0.005  ms/op
>> LoopOverNonConstantHeap.segment_loop_slice                          false  avgt   30  0.203 ± 0.002  ms/op
>> LoopOverNonConstantHeap.segment_loop_slice                           true  avgt   30  0.207 ± 0.004  ms/op
>> LoopOverNonConstantHeap.segment_loop_unaligned                      false  avgt   30  0.206 ± 0.004  ms/op
>> LoopOverNonConstantHeap.segment_loop_unaligned                       true  avgt   30  0.209 ± 0.003  ms/op
>> LoopOverNonConstantHeap.unsafe_get                                  false  avgt   30  0.386 ± 0.017  ns/op
>> LoopOverNonConstantHeap.unsafe_get                                   true  avgt   30  0.381 ± 0.017  ns/op
>> Loo...
>
> Great work @liach -- I've bumped the number of reviewers as this whole area is rather tricky, and I think the PR can probably benefit from another independent pass.

> Unfortunately no. I don't really know about the Foreign Memory API as I only started to investigate its performance bottlenecks recently. @mcimadamore or @JornVernee were more involved in the development and likely know better.

After a chat with @PaulSandoz , it seems like the exclusion was not a deliberate one. Probably, sub-int atomic unsafe primitives were added in parallel with the var handle work:

https://github.com/openjdk/jdk/commit/e663206d

The above commit did add support for sub-word atomics, but only in the field access case. The BB view/array view var handles were left out. Then came FFM, whose implementation was originally based off the BB view var handle -- so we ended up inheriting same limitations.

It would be good to address this -- in a separate PR. As for this PR, as we discussed offline, I'd suggest to keep the ScopedMemoryAccess changes (even if some methods are now unused) and drop the commented lines from the var handle gensrc file. Then we can come back with a new PR and add support for missing access modes.

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

PR Comment: https://git.openjdk.org/jdk/pull/23720#issuecomment-2690284867