Implement ROMMethodInfo caching support and stack walker integration by harship04 · Pull Request #22918 · eclipse-openj9/openj9

harship04 · 2025-11-11T10:04:39Z

Implements a unified ROM method caching mechanism for consolidating ROM method metadata. Updates stack walking to use the cached information instead of repeated metadata lookups.

gacholio · 2025-11-13T14:14:38Z

Waiting for the changes we discussed before review (but at a glance, looks good so far).

harship04 · 2025-11-17T05:20:05Z

@gacholio , could you please review the PR

gacholio · 2025-11-19T15:14:24Z

runtime/vm/swalk.c

 	MARK_SLOT_AS_OBJECT(walkState, (j9object_t*) &(methodFrame->specialFrameFlags));
 	walkState->method = methodFrame->method;
+	if (NULL != walkState->method) {
+		initializeBasicROMMethodInfo(walkState, J9_ROM_METHOD_FROM_RAM_METHOD(walkState->method));


I don't think there should be any remaining callers of the basic init code (other than internally in mapcache.cpp) - they should all be replaced with the new ROM method call (other than the one for a bytecoded PC of course).

runtime/stackmap/mapcache.cpp

gacholio · 2025-11-19T15:21:05Z

runtime/stackmap/mapcache.cpp

+	/* Always compute argument bits */
+	j9localmap_ArgBitsForPC0(romClass, romMethod, newInfo.argbits);
+
+	if (computeStackAndLocals && pc < (UDATA)J9_BYTECODE_SIZE_FROM_ROM_METHOD(romMethod)) {


Please bracket the second clause. In fact, why is this necessary? I think we can assume a running PC is withing the bytecode range of its method.

I removed the pc < (UDATA)J9_BYTECODE_SIZE_FROM_ROM_METHOD(romMethod) check in populateROMMethodInfo, but when I also remove the corresponding condition in getROMMethodInfoForBytecodePC—the one that returns if (pc <= J9SF_MAX_SPECIAL_FRAME_TYPE || pc >= (UDATA)J9_BYTECODE_SIZE_FROM_ROM_METHOD(romMethod)) then the JVM crashes with a segmentation faul

gacholio · 2025-11-19T15:23:00Z

runtime/stackmap/mapcache.cpp

+				NULL);
+	}
+
+	/* Copy metadata */


Should this not be a call to the basic init code?

gacholio · 2025-11-19T15:24:10Z

Looks good in general. When computing the maps, you are not checking the size, checking for failure, or setting the bits indicating that the maps have been cached.

gacholio · 2025-11-19T15:25:08Z

The argbits do not seem to be being computed at all.

runtime/stackmap/mapcache.cpp

harship04 · 2025-12-01T14:10:46Z

The segmentation fault occurred because when pc == 0, the code returned early without initializing walkState->romMethodInfo; this was fixed by calling initializeBasicROMMethodInfo() for special-frame or out-of-bounds PC values.

harship04 · 2025-12-01T14:11:02Z

@gacholio

I added computePendingCountForBytecode() for bytecode-PC frames (argCount + tempCount, plus 1 for synchronized or non-empty methods).
Could you please confirm if this computation is correct, and whether the pending count should be updated inside populateROMMethodInfo() or only used when populating the cache

J9OSRFrame has pendingStackHeight and numberOfLocals, but J9ROMMethodInfo doesn’t store them. Currently, populateROMMethodInfo() receives these values for OSR frames but doesn’t save them. Should ROMMethodInfo eventually store them for caching, or are they only needed during map computation?

harship04 · 2026-03-09T15:01:05Z

issue:
The original stack walker had two layers of protection preventing NULL constant pool dereferences. First, the constant pool was only accessed inside the J9_STACKWALK_ITERATE_O_SLOTS block, which is set only during GC root scanning and object slot enumeration. So for special frames (where constantPool == NULL), that entire block was skipped. Second, even when the flag was set, the code checked if (walkState->argCount), and special frames typically have argCount == 0, so the inner logic was skipped again. This maintained the invariant that only frames safe for object slot enumeration ever dereference the constant pool.
In the new implementation, getROMMethodInfoForROMMethod() is called unconditionally, outside the J9_STACKWALK_ITERATE_O_SLOTS check. Since that function internally dereferences class/method data derived from the constant pool, it now runs even during walks of frames where the constant pool is NULL—breaking the original protection and causing the crash.

original code:

if (walkState->method) {
		J9ROMMethod * romMethod = J9_ROM_METHOD_FROM_RAM_METHOD(walkState->method);

		walkState->constantPool = UNTAGGED_METHOD_CP(walkState->method);
		walkState->argCount = J9_ARG_COUNT_FROM_ROM_METHOD(romMethod);

		if (walkState->flags & J9_STACKWALK_ITERATE_O_SLOTS) {
			WALK_METHOD_CLASS(walkState);

			if (walkState->argCount) {
				/* Max size as argCount always <= 255 */
				U_32 result[8];
				J9Class *methodClass = UNTAGGED_METHOD_CP(walkState->method)->ramClass;

#ifdef J9VM_INTERP_STACKWALK_TRACING
				swPrintf(walkState, 4, "\tUsing signature mapper\n");
#endif
				j9cached_ArgBitsForPC0(methodClass->romClass, romMethod, result, walkState->javaVM, methodClass->classLoader);

#ifdef J9VM_INTERP_STACKWALK_TRACING
				swPrintf(walkState, 4, "\tArguments starting at %p for %d slots\n", walkState->arg0EA, walkState->argCount);
#endif
				walkState->slotType = J9_STACKWALK_SLOT_TYPE_METHOD_LOCAL;
				walkState->slotIndex = 0;
				if (walkState->frameFlags & J9_SSF_JNI_REFS_REDIRECTED) {
					walkIndirectDescribedPushes(walkState, walkState->arg0EA, walkState->argCount, result);
				} else {
					walkDescribedPushes(walkState, walkState->arg0EA, walkState->argCount, result, walkState->argCount);
				}
			}
		}

the method causing the issue will null cp

method=0x7f952e7080a0 constantPool=(nil) pc=0x2 flags=0x1c0c0100 frameFlags=0x0
romMethod=0x7f952e70d4ec modifiers=0x0 argCount=0

It seems that the code never reached the line that dereferences the constant pool—J9Class *methodClass = UNTAGGED_METHOD_CP(walkState->method)->ramClass—because the frame never passes the checks if (walkState->flags & J9_STACKWALK_ITERATE_O_SLOTS) or if (walkState->argCount).

harship04 · 2026-03-09T15:03:26Z

@gacholio as we discussed, I’ve updated the code . Could you please review it.

To safely determine the class loader for caching, I deconstructed the J9_CLASS_FROM_METHOD macro by explicitly retrieving the constant pool using J9_CP_FROM_METHOD(method) and checking if it is NULL. If the constant pool is NULL, the bootstrap loader (vm->systemClassLoader) is used, since such methods are internal VM frames not loaded from class files.
In getROMMethodInfoCommon(), I added an argCount > 0 check so that the argument mapper is not called when there are no arguments. And after this change, can I proceed with checking for performance regressions?

runtime/codert_vm/jswalk.c

runtime/oti/j9nonbuilder.h

runtime/oti/stackmap_api.h

runtime/stackmap/mapcache.cpp

keithc-ca · 2026-03-09T15:25:00Z

runtime/stackmap/mapcache.cpp

+        /* Only compute argbits if method has arguments */
+        if (romMethodInfo->argCount > 0) {
+                J9ROMClass *romClass = J9_CLASS_FROM_METHOD(method)->romClass;
+                if (romMethodInfo->argCount <= (J9_ARGBITS_CACHE_SLOTS * 32)) {


What does 32 mean here?

#define J9_STACKMAP_CACHE_SLOTS 2 #define J9_LOCALMAP_CACHE_SLOTS 2 #define J9_ARGBITS_CACHE_SLOTS 2 /* Flag values for J9ROMMethodInfo */ #define J9MAPCACHE_STACKMAP_CACHED 1 #define J9MAPCACHE_LOCALMAP_CACHED 2 #define J9MAPCACHE_ARGBITS_CACHED 4 #define J9MAPCACHE_METHOD_IS_CONSTRUCTOR 8 #define J9MAPCACHE_VALID 128 /* J9ROMMethodInfo must be a multiple of 8 bytes in size */ typedef struct J9ROMMethodInfo { void *key; U_32 stackmap[J9_STACKMAP_CACHE_SLOTS]; U_32 localmap[J9_ARGBITS_CACHE_SLOTS]; U_32 argbits[J9_ARGBITS_CACHE_SLOTS]; U_32 modifiers;

32 represents the number of bits in a U_32. Since argbits is stored as an array of U_32, each element can hold 32 argument reference bits.The conditionromMethodInfo->argCount <= (J9_ARGBITS_CACHE_SLOTS * 32)checks whether the number of method arguments can fit in the argbits bitmap used for caching. The argbits field is an array of U_32, and each U_32 contains 32 bits, where each bit represents one argument slot. Since J9_ARGBITS_CACHE_SLOTS is 2, the bitmap can store up to 2 × 32 = 64 argument bits.

Instead of J9_*_CACHE_SLOTS, perhaps we should have:

/* Cache slot sizes must all be multiples of 64 bits. */ #define J9_STACKMAP_CACHE_BITS 64 #define J9_LOCALMAP_CACHE_BITS 64 #define J9_ARGBITS_CACHE_BITS 64

typedef struct J9ROMMethodInfo { void *key; U_32 stackmap[J9_STACKMAP_CACHE_BITS / sizeof(U_32)]; U_32 localmap[J9_ARGBITS_CACHE_BITS / sizeof(U_32)]; U_32 argbits[J9_ARGBITS_CACHE_BITS / sizeof(U_32)];

Then line 130 (above) becomes:

if (romMethodInfo->argCount <= J9_ARGBITS_CACHE_BITS) {

@keithc-ca ,
Based on my understanding, sizeof(U_32) returns 4 bytes, and sizeof(U_32) * 8 gives 32 bits. If we compute the array size using J9_ARGBITS_CACHE_BITS / sizeof(U_32), it becomes 64 / 4 which is 16.
and if we do this J9_STACKMAP_CACHE_BITS / (sizeof(U_32) * 8) then it will be 64/32 which will be 2 slots
Could you please confirm if this is correct, or if I should keep the original definition using J9_STACKMAP_CACHE_SLOTS 2 instead?

Sorry, you're right, those arrays should use / 32 instead of / sizeof(U_32):

U_32 stackmap[J9_STACKMAP_CACHE_BITS / 32]; U_32 localmap[J9_ARGBITS_CACHE_BITS / 32]; U_32 argbits[J9_ARGBITS_CACHE_BITS / 32];

runtime/oti/j9nonbuilder.h

runtime/oti/stackmap_api.h

runtime/stackmap/mapcache.cpp

runtime/vm/swalk.c

runtime/stackmap/mapcache.cpp

harship04 · 2026-03-17T14:10:47Z

@gacholio, We initially assumed that there was no need to check the PC offset, since the stack walker already determines the frame type and calls the bytecode frame helper.
when I removed the following condition it resulted in a crash.

if (pcOffset >= (UDATA)J9_BYTECODE_SIZE_FROM_ROM_METHOD(romMethod)) {
    initializeBasicROMMethodInfo(walkState, romMethod);
    romMethodInfo->key = (void *)romMethod;
    return;
}

I observed that pcOffset can still be invalid ( very large and outside the bytecode range) because getROMMethodInfoForBytecodeFrame is invoked from jitWalkStackFrames in file runtime/codert_vm/jswalk.c even when the PC does not correspond to a valid bytecode location.

This leads to an out-of-bounds bytecodePC being computed and passed into the stackmap logic, which assumes a valid bytecode PC and results in a segmentation fault .

keithc-ca · 2026-03-17T16:46:24Z

runtime/oti/j9nonbuilder.h

+	U_32 stackmap[J9_STACKMAP_CACHE_BITS / 32];
+	U_32 localmap[J9_LOCALMAP_CACHE_BITS / 32];
+	U_32 argbits[J9_ARGBITS_CACHE_BITS / 32];


These fields should use name patterns that are consistent:

U_32 stackMap[J9_STACKMAP_CACHE_BITS / 32]; U_32 localMap[J9_LOCALMAP_CACHE_BITS / 32]; U_32 argBits[J9_ARGBITS_CACHE_BITS / 32];

This has not been addressed.

This is a work in progress.

Sorry for the delay, I was on leave.
@keithc-ca, Changes are now updated.

gacholio · 2026-03-20T01:38:57Z

getROMMethodInfoForBytecodeFrame is invoked from jitWalkStackFrames

This is the issue - JIT frames are not bytecode frames. This call should be to the ROMMethod entrypoint (JIT frames do not use the computed stack/local maps - they only require the argbits). This lines up with the use of the ROM method as key for the other frames (native method, etc).

Signed-off-by: Graham Chapman <[email protected]>

harship04 · 2026-03-21T04:03:25Z

getROMMethodInfoForBytecodeFrame is invoked from jitWalkStackFrames

This is the issue - JIT frames are not bytecode frames. This call should be to the ROMMethod entrypoint (JIT frames do not use the computed stack/local maps - they only require the argbits). This lines up with the use of the ROM method as key for the other frames (native method, etc).

@gacholio I’ve replaced the call with:

J9ROMMethod *romMethod = J9_ROM_METHOD_FROM_RAM_METHOD(walkState->method);
getROMMethodInfoForROMMethod(walkState, romMethod);

so that JIT frames now use the ROMMethod entrypoint instead of the bytecode frame helper.

harship04 force-pushed the romcache branch 9 times, most recently from 19d1ee8 to 5f11894 Compare November 11, 2025 15:38

babsingh requested a review from gacholio November 11, 2025 16:19

harship04 force-pushed the romcache branch 3 times, most recently from 24bce23 to 18af487 Compare November 15, 2025 12:33

gacholio reviewed Nov 19, 2025

View reviewed changes

runtime/stackmap/mapcache.cpp Outdated Show resolved Hide resolved

gacholio reviewed Nov 19, 2025

View reviewed changes

harship04 force-pushed the romcache branch 2 times, most recently from 495e826 to b581cfd Compare November 25, 2025 20:26

gacholio reviewed Nov 26, 2025

View reviewed changes

runtime/stackmap/mapcache.cpp Outdated Show resolved Hide resolved

harship04 force-pushed the romcache branch from b581cfd to 54734e9 Compare November 28, 2025 11:33

harship04 force-pushed the romcache branch 3 times, most recently from e5bafb7 to 9ae4274 Compare January 8, 2026 14:17

keithc-ca requested changes Mar 9, 2026

View reviewed changes

harship04 force-pushed the romcache branch 2 times, most recently from 37d7dad to 7629b32 Compare March 10, 2026 12:58

harship04 requested a review from keithc-ca March 10, 2026 13:59

keithc-ca requested changes Mar 10, 2026

View reviewed changes

harship04 marked this pull request as draft March 11, 2026 13:41

harship04 force-pushed the romcache branch from 7629b32 to 8874b16 Compare March 11, 2026 16:35

harship04 marked this pull request as ready for review March 11, 2026 16:52

keithc-ca reviewed Mar 11, 2026

View reviewed changes

runtime/stackmap/mapcache.cpp Outdated Show resolved Hide resolved

harship04 marked this pull request as draft March 12, 2026 16:13

harship04 force-pushed the romcache branch 2 times, most recently from d50ed41 to 4129a78 Compare March 17, 2026 06:11

harship04 marked this pull request as ready for review March 17, 2026 14:12

harship04 requested a review from keithc-ca March 17, 2026 16:13

keithc-ca reviewed Mar 17, 2026

View reviewed changes

gacholio marked this pull request as draft March 20, 2026 18:57

gacholio and others added 5 commits March 20, 2026 20:34

Part 1 of stack walker ROM method cache

a36f157

Signed-off-by: Graham Chapman <[email protected]>

Implement ROMMethodInfo caching support in mapcache

71c20d0

fixup

1577ef1

Add third entry point getROMMethodInfoForOSRFrame

c3de641

fixup

913c462

harship04 force-pushed the romcache branch from 5686a74 to 9d8611e Compare March 21, 2026 03:34

fixup

2ce0ec6

harship04 force-pushed the romcache branch from 9d8611e to 2ce0ec6 Compare March 21, 2026 04:30

harship04 marked this pull request as ready for review March 21, 2026 04:42

Conversation

harship04 commented Nov 11, 2025

Uh oh!

gacholio commented Nov 13, 2025

Uh oh!

harship04 commented Nov 17, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

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!

gacholio commented Nov 19, 2025

Uh oh!

gacholio commented Nov 19, 2025

Uh oh!

Uh oh!

harship04 commented Dec 1, 2025

Uh oh!

harship04 commented Dec 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

harship04 commented Mar 9, 2026

Uh oh!

harship04 commented Mar 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

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!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

harship04 commented Mar 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

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!

harship04 Mar 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gacholio commented Mar 20, 2026

Uh oh!

harship04 commented Mar 21, 2026

Uh oh!

harship04 commented Dec 1, 2025 •

edited

Loading

harship04 commented Mar 9, 2026 •

edited

Loading

harship04 commented Mar 17, 2026 •

edited

Loading

harship04 Mar 21, 2026 •

edited

Loading