RFR: 8350118: Simplify the layout access VarHandle

[Chen Liang]

On Fri, 21 Feb 2025 09:59:27 GMT, Maurizio Cimadamore <mcimadamore at openjdk.org> wrote:

>> Simplify the layout access var handles to be direct in some common cases. Also made `VarHandle::isAccessModeSupported` report if an access mode is supported for a VH.
>> 
>> Reduces the instructions to execute this code in a simple main by 47%:
>> 
>> long[] arr = new long[8];
>> var ms = MemorySegment.ofArray(arr);
>> ms.setAtIndex(ValueLayout.JAVA_BYTE, 12, (byte) 3);
>> 
>> 
>> Main overheads in FFM are identified to be:
>> 1. Eager initialization of direct MethodHandle; can be CDS archived
>> 2. MH combinator forms via LambdaFormEditor, not cached right now and always have large overhead
>> 
>> Still need other measures to deal with common user patterns of `MethodHandles.insertCoordinates(vh, 1, 0L)` which currently is still very slow.
>> 
>> Tests: 2 unrelated failures on tier 1-3
>
> make/modules/java.base/gensrc/GensrcVarHandles.gmk line 171:
> 
>> 169: # Param 1 - Variable declaration prefix
>> 170: # Param 2 - Type with first letter capitalized
>> 171: # TODO decide if CAS/Bitwise/AtomicAdd should be enabled for subints
> 
> Exactly - let's keep this out for now (they seem to be currently enabled -- and I'd prefer to do that separately, after some discussion)

I have disabled them with #, and the status is confirmed by test for access modes. I kept the infra to make future reenabling easy.

> src/java.base/share/classes/java/lang/invoke/MethodHandleImpl.java line 1556:
> 
>> 1554: 
>> 1555:             @Override
>> 1556:             public VarHandle memorySegmentViewHandle(Class<?> carrier, MemoryLayout enclosing, long alignmentMask, ByteOrder order, boolean fixedOffset, long offset) {
> 
> When I was playing with the code I kept being confused by the `fixedOffset` parameter name. The reason being that no var handle is really "fixed offset" (all VH take a "base" offset -- and this doesn't change that). What this "fixedOffset" means is really "there's no other (dynamic) offset on top of the base offset". I think calling it "stridedAccess" seems subjectively more evocative (or something like that).

Shouldn't this be called noStride then?

> src/java.base/share/classes/java/lang/invoke/VarHandles.java line 767:
> 
>> 765: //                    new HandleType(Object.class, long.class),
>> 766: //
>> 767: //                    // MS[base][offset]->T
> 
> I like how you programmatically generate all guards for all the shapes we care about.
> 
> On `MS[base][offset]` I wonder if generating a guard matters (for now) given that such a var handle will be indirect?

Yes, it avoids the extra generation of vh linker lambda forms.

> src/java.base/share/classes/jdk/internal/foreign/Utils.java line 77:
> 
>> 75: 
>> 76:     private static MethodHandle computeFilterHandle(int index) {
>> 77:         MethodHandles.Lookup lookup = MethodHandles.lookup();
> 
> please break this big conditional up!

This is a one-time cold path initializer so I did not care too much about its size. How should I break it up?

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

PR Review Comment: https://git.openjdk.org/jdk/pull/23720#discussion_r1965506674
PR Review Comment: https://git.openjdk.org/jdk/pull/23720#discussion_r1965508406
PR Review Comment: https://git.openjdk.org/jdk/pull/23720#discussion_r1965504398
PR Review Comment: https://git.openjdk.org/jdk/pull/23720#discussion_r1965512480