Fold MemorySegment primitive get/set into direct accesses on JDK 21+ by nbhuiyan · Pull Request #24304 · eclipse-openj9/openj9

nbhuiyan · 2026-07-07T21:38:11Z

MemorySegment.get and MemorySegment.set methods for byte, char, short, int, long, float and double data types dispatch through a deep VarHandle / MethodHandle / LambdaForm / VarHandleSegmentAsX / ScopedMemoryAccess chain. Even when such call chains inline fully, they leave a sequence of guards, such as LambdaForm dispatch, segment accessors and session checks, each of which cost a compare and branch per access and cannot be hoisted out of loops.

This commit implements direct lowering of these accessors in J9RecognizedCallTransformer during its early pass (part of ILGen opts) to a guarded direct load or store at segment.min + offset whenever the layout argument is known at compile time. The per-access runtime overhead of the slow chain is replaced by compile-time folding of the layout's byte order and alignment constraint plus a single combined guard.

The guard ORs together every condition the interpreted path checks and branches to the original call when any of them fails:

receiver is NativeMemorySegmentImpl or MappedMemorySegmentImpl (heap segments take the slow path)
0 <= offset <= length - accessSize (bounds check)
min + offset aligned to the layout's byteAlignment, for alignment-constrained layouts (folded away for *_UNALIGNED layouts)
scope state is read through a volatile load, so the check is neither hoisted nor commoned across iterations
owner == null || owner == currentThread
!readOnly (for stores)

The slow path is the unmodified original call, so each failing guard preserves the exact semantics and throws exceptions as necessary. Reversed-order layouts are handled by a compile-time endian conversion (a byte swap on the value, float and double go through their integer bit patterns).

MemorySegment.get/set are given fine-grained recognized method ids so the transformation can match each primitive overload, and J9EstimateCodeSize keeps treating them as MemorySegment methods so the InterpreterEmulator's stateful bytecode iteration still folds accessHandle().

Two VM frontend queries, TR_J9VMBase::getLayoutByteOrder andgetLayoutByteAlignment, that read the known layout object's order and byteAlignment have been implemented for this transformation. Corresponding JITServer changes have also been implemented.

This transformation requires additional work for AOT support, so it is currently skipped during AOT compilations. A new env option TR_disableFFMDirectLowering can be used to disable the transformation.

Correctness of the fast path rests on three invariants:

the scope-state load is volatile
there is no yield point between that load and the access
native segment memory is freed only after ScopedMemoryAccess.closeScope0 has taken exclusive VM access A racing close therefore either lets an in-flight access complete before the thread gets to a yield point, or is observed as a closed state just prior the next access, which results in fallback to the slow path. This also covers Cleaner-managed implicit arenas without a reachabilityFence on the fast path.

This is the final part of the 3-part MemorySegment improvement work. Other two parts:

MemorySegment.get and MemorySegment.set methods for byte, char, short, int, long, float and double data types dispatch through a deep VarHandle / MethodHandle / LambdaForm / VarHandleSegmentAsX / ScopedMemoryAccess chain. Even when such call chains inline fully, they leave a sequence of guards, such as LambdaForm dispatch, segment accessors and session checks, each of which cost a compare and branch per access and cannot be hoisted out of loops. This commit implements direct lowering of these accessors in J9RecognizedCallTransformer during its early pass (part of ILGen opts) to a guarded direct load or store at segment.min + offset whenever the layout argument is known at compile time. The per-access runtime overhead of the slow chain is replaced by compile-time folding of the layout's byte order and alignment constraint plus a single combined guard. The guard ORs together every condition the interpreted path checks and branches to the original call when any of them fails: - receiver is NativeMemorySegmentImpl or MappedMemorySegmentImpl (heap segments take the slow path) - 0 <= offset <= length - accessSize (bounds check) - min + offset aligned to the layout's byteAlignment, for alignment-constrained layouts (folded away for *_UNALIGNED layouts) - scope state is read through a volatile load, so the check is neither hoisted nor commoned across iterations - owner == null || owner == currentThread - !readOnly (for stores) The slow path is the unmodified original call, so each failing guard preserves the exact semantics and throws exceptions as necessary. Reversed-order layouts are handled by a compile-time endian conversion (a byte swap on the value, float and double go through their integer bit patterns). MemorySegment.get/set are given fine-grained recognized method ids so the transformation can match each primitive overload, and J9EstimateCodeSize keeps treating them as MemorySegment methods so the InterpreterEmulator's stateful bytecode iteration still folds accessHandle(). Two VM frontend queries, TR_J9VMBase::getLayoutByteOrder andgetLayoutByteAlignment, that read the known layout object's order and byteAlignment have been implemented for this transformation. Corresponding JITServer changes have also been implemented. This transformation requires additional work for AOT support, so it is currently skipped during AOT compilations. A new env option TR_disableFFMDirectLowering can be used to disable the transformation. Correctness of the fast path rests on three invariants: * the scope-state load is volatile * there is no yield point between that load and the access * native segment memory is freed only after ScopedMemoryAccess.closeScope0 has taken exclusive VM access A racing close therefore either lets an in-flight access complete before the thread gets to a yield point, or is observed as a closed state just prior the next access, which results in fallback to the slow path. This also covers Cleaner-managed implicit arenas without a reachabilityFence on the fast path. Signed-off-by: Nazim Bhuiyan <nubhuiyan@ibm.com>

nbhuiyan · 2026-07-07T21:41:42Z

@vijaysun-omr @mpirvu @0xdaryl Requesting review from at least two of you.

vijaysun-omr · 2026-07-09T20:10:49Z

+
+    // ---  Compare tree: OR'd compound guard ---
+    TR::SymbolReference *vftSR = srTab->findOrCreateVftSymbolRef();
+    TR::Node *segVft = TR::Node::createWithSymRef(node, TR::aloadi, 1, segmentNode->duplicateTree(), vftSR);


should we emit a test for null on the segmentNode value before we indirect ?

I did not think that was necessary because we already had bounds checking earlier, binding segmentNode to <= requiredChildren. createTempsForCalls would also have iterated over the call node's children and dereferenced segmentNode child.

sorry I meant a null test in generated code, not a null test at compile time

Yes, a NULLCHK on the segmentNode is inserted just before the diamond. See Lines 634 onwards:

// NULLCHK on the segment before the diamond: it must precede the vft load // in the guard, and the segment has to be non-null on the slow path too. TR::SymbolReference *nullChkSR = srTab->findOrCreateNullCheckSymbolRef(comp()->getMethodSymbol()); TR::Node *passThrough = TR::Node::create(node, TR::PassThrough, 1, segmentNode->duplicateTree()); TR::Node *nullChk = TR::Node::createWithSymRef(node, TR::NULLCHK, 1, passThrough, nullChkSR); treetop->insertBefore(TR::TreeTop::create(comp(), nullChk));

Thanks, I saw that after I posted my comment. I assume it is okay to raise an actual exception at this point since the original call would have raised such an exception (NPE) as well.

vijaysun-omr · 2026-07-09T20:18:24Z

+
+    // --- Fast tree: direct access at min + offset ---
+    TR::Node *minLoad = TR::Node::createWithSymRef(node, TR::lloadi, 1, segmentNode->duplicateTree(), minSR);
+    TR::Node *addr = TR::Node::create(node, TR::ladd, 2, minLoad, offsetNode->duplicateTree());


duplicateTree can be a tricky api in that it duplicates the subtree to be evaluated at the new location, so if any nodes were anchored and the relevant symbols had their values changed, the calling it here could result in a new load being created instead of getting the anchored value, thus causing different semantics

Maybe this is impossible here because there are temp loads created and they are known not to change in the interim

I had to implement similar transformations in the past and used duplicateTree, where I got to learn (through trial and error) about the importance of first creating temps for call before duplicating. Is there something additional to be done to ensure we don't run into issues in the future?

that is good, and probably sufficient if you can convince yourself that there will not be any commoning of a load across a store of the temp in question....i.e.

load x ... store x ... rhs ... ==> load x

situation cannot happen.

vijaysun-omr · 2026-07-09T20:22:59Z

Given the long sequence of tree transformations for the different API cases being handled, I think a question has to be asked about the kind of unit testing possible/done here. It is going to be hard to spot bugs in the sequence of transformations and we should probably make sure every API case is at least exercised.

nbhuiyan · 2026-07-10T16:33:32Z

Given the long sequence of tree transformations for the different API cases being handled, I think a question has to be asked about the kind of unit testing possible/done here. It is going to be hard to spot bugs in the sequence of transformations and we should probably make sure every API case is at least exercised.

I have exercised all of these transformations through a microbenchmark I used to analyze performance. With the help of IBM Bob, I have also created a set of verification tests (derived from existing openjdk tests) that confirm that that guard-failure conditions are properly set up and we maintain the expected behaviour through fallback to the slow path.

As for the existing coverage, all of the different data types are similar in semantics, so tests that cover a subset of the data types effectively cover all. We do have an extensive amount of coverage as part of the openjdk test targets (the sources for them located under test/jdk/java/foreign that exercise nearly all of these cases being handled here. For example, see: TestHandshake.java, TestMemoryDereference.java, plus others in the same package. Given the extensive amount of existing test coverage, I did not feel there was a need for adding any additional tests.

vijaysun-omr · 2026-07-10T19:48:05Z

jenkins test sanity.functional all jdk25

nbhuiyan · 2026-07-10T20:37:53Z

I would recommend running sanity.openjdk test as well, as that exercises the transformer.

vijaysun-omr · 2026-07-10T23:34:43Z

jenkins test sanity.openjdk all jdk25

nbhuiyan · 2026-07-13T15:48:41Z

There are many failed tests. From a few failures I looked into at random, they have all been related to const-refs (#24336). Given the large number of failures and the fact that I am also working on resolving #24336, I think we should hold off and re-launch the PR testing later.

nbhuiyan · 2026-07-13T18:00:05Z

jenkins test sanity.openjdk all jdk25 depends eclipse-omr/omr#8337

nbhuiyan · 2026-07-13T19:05:45Z

I have relaunched tests with the potential fix for #24336 included in the build to remove all the constrefs-related noise. In the latest set of build, I have not seen any const-refs related failures yet. However, I am seeing a new failure that is probably directly a result of the changes:

sanity.openjdk aarch64 mac failure:

14:31:01  Accessor #7 terminated - elapsed (ms): 134
14:31:01  test TestHandshake.testHandshake("BufferHandleAccessor", TestHandshake$$Lambda/0x000000007006c6e8@5d7340a1): success [884ms]
14:31:01  
14:31:01  ===============================================
14:31:01  java/foreign/TestHandshake.java
14:31:01  Total tests run: 7, Passes: 7, Failures: 0, Skips: 0
14:31:01  ===============================================
14:31:01  
14:31:01  STDERR:
14:31:01  Assertion failed at /Users/jenkins/workspace/Build_JDK25_aarch64_mac_Personal/openj9/runtime/compiler/compile/J9SymbolReferenceTable.cpp:761: sym->isVolatile() || !isVolatile
14:31:01  VMState: 0x00053bff
14:31:01  	expecting volatile symref but found non-volatile symref #1876
14:31:01  
14:31:01  compiling TestHandshake$AbstractSegmentAccessor.run()V at level: very-hot (profiling)

I have not seen this failure in any of my testing, so this needs investigating.

nbhuiyan · 2026-07-13T19:53:51Z

sanity.openjdk x86-64 Linux contains the symref-related failure seen in aarch64 mac, and contains an additional failure that requires investigating:

14:50:31  test TestSegmentCopy.testByteCopySizes(NATIVE, ARRAY): success [43ms]
14:50:31  stderr:
14:50:31  00007F50E070B100: Object neither in heap nor stack-allocated in thread AgentVMThread
14:50:31  00007F50E070B100:	O-Slot=00007F501800A7A0
14:50:31  00007F50E070B100:	O-Slot value=00007F5018213C00
14:50:31  00007F50E070B100:	PC=00007F5056F7137D
14:50:31  00007F50E070B100:	framesWalked=2
14:50:31  00007F50E070B100:	arg0EA=00007F501800A810
14:50:31  00007F50E070B100:	walkSP=00007F501800A578
14:50:31  00007F50E070B100:	literals=0000000000000000
14:50:31  00007F50E070B100:	jitInfo=00007F50AE1DA638
14:50:31  00007F50E070B100:	method=00007F50180202B8 (TestSegmentCopy.testByteCopySizes(LTestSegmentCopy$SegmentKind;LTestSegmentCopy$SegmentKind;)V) (JIT)
14:50:31  00007F50E070B100:	stack=00007F5018004398-00007F501800B420
14:50:31  18:50:27.531 0x7f503c003c00    j9mm.479    *   ** ASSERTION FAILED ** at /home/jenkins/workspace/Build_JDK25_x86-64_linux_Personal/openj9/runtime/gc_glue_java/ScavengerRootScanner.hpp:108: ((MM_StackSlotValidator(MM_StackSlotValidator::NOT_ON_HEAP, *slotPtr, stackLocation, walkState).validate(_env)))

All other failures I have seen so far on other platforms is an instance of these two distinct problems.

nbhuiyan requested a review from dsouzai as a code owner July 7, 2026 21:38

nbhuiyan added comp:jit project:panama Used to track Project Panama related work project:MH Used to track Method Handles related work labels Jul 7, 2026

nbhuiyan removed the request for review from dsouzai July 7, 2026 21:40

vijaysun-omr reviewed Jul 9, 2026

View reviewed changes

This was referenced Jul 13, 2026

Assertion failed J9CodeGenerator.cpp:5290 VMState: 0x0005ff09 the const provenance graph is missing at least one edge #24336

Open

Fix createTempsForCall's handling of not-collected references #24375

Open

Uh oh!

Conversation

nbhuiyan commented Jul 7, 2026

Uh oh!

nbhuiyan commented Jul 7, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vijaysun-omr commented Jul 9, 2026

Uh oh!

nbhuiyan commented Jul 10, 2026

Uh oh!

vijaysun-omr commented Jul 10, 2026

Uh oh!

nbhuiyan commented Jul 10, 2026

Uh oh!

vijaysun-omr commented Jul 10, 2026

Uh oh!

nbhuiyan commented Jul 13, 2026

Uh oh!

nbhuiyan commented Jul 13, 2026

Uh oh!

nbhuiyan commented Jul 13, 2026

Uh oh!

nbhuiyan commented Jul 13, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants