8375719: [lworld] Move reads of flat fields entirely to the runtime #1936

jsikstro · 2026-01-20T17:56:00Z

Hello,

Today much of the reading of flattened values in the interpreter is implemented both in the platform-specific templateTable_XX.cpp files, and also in the runtime via InterpreterRuntime::read_flat_field. The reason we have both is becuase the interpreterlet implements a fast-path for null-free types, which attempts to allocate the buffered object inside the thread's TLAB, before moving on to copy the src payload to the buffered objected. The copying is done in the VM via a call_VM_leaf call, which notably does not safepoint-poll (nor allow anything that might GC for example).

The slow-path in the interpreterlet calls into the VM via a call_VM, which notably safepoint-point polls upon exit. The slow-path is taken when 1) the src payload is nullable or 2) the fast-path TLAB allocation fails.

I propose we redesign the dance around when and how to enter the VM by having the logic of reading a flat field exclusively inside the runtime and thus always entering the VM. This approach allows us to have one canonical way to read flat fields, without having effectively duplicate code in interpreterlets (for all supported platforms) and inside the runtime. A benefit from this is that it becomes easier for porters to port the Valhalla changes to their platform(s) and later on maintain that port, which is a plus.

Since all objects are NULLABLE in JEP 401 (disregarding F&F interface), all reads of flat fields are already entering the VM via the "slow-path". This means that this change only has a practical effect on null-free/null-restricted fields. I think we should consider if having a fast-path is worth it or not when that time comes, although I anticipate that it doesn't make much difference since the copy is always done in the VM anyway.

As a small optimization, I've added a check for nullable and marked-as-null before entering the VM.

Testing:

hotspot_valhalla, jdk_valhalla and Oracle's tier1-4 on linux-x64-debug and linux-aarch64-debug

Progress

Change must not contain extraneous whitespace
Commit message must refer to an issue
Change must be properly reviewed (1 review required, with at least 1 Committer)

Issue

JDK-8375719: [lworld] Move reads of flat fields entirely to the runtime (Enhancement - P4)

Reviewers

Coleen Phillimore (@coleenp - Committer) ⚠️ Review applies to 61c1f4cf
Paul Hübner (@Arraying - Committer)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/valhalla.git pull/1936/head:pull/1936
$ git checkout pull/1936

Update a local copy of the PR:
$ git checkout pull/1936
$ git pull https://git.openjdk.org/valhalla.git pull/1936/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 1936

View PR using the GUI difftool:
$ git pr show -t 1936

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/valhalla/pull/1936.diff

Using Webrev

Link to Webrev Comment

bridgekeeper · 2026-01-20T17:56:59Z

👋 Welcome back jsikstro! A progress list of the required criteria for merging this PR into lworld will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

openjdk · 2026-01-20T17:58:47Z

@jsikstro This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

8375719: [lworld] Move reads of flat fields entirely to the runtime

Reviewed-by: coleenp, phubner

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 3 new commits pushed to the lworld branch:

fa557ef: 8375740: [lworld] javap prints "value class" while class flags indicate ACC_IDENTITY
a446415: 8375328: [lworld] JNI IsSameObject function doesn't use the alternate substitutability method
4c6f902: 8375602: [lworld] Test verifiers should verify the equivalence of the objects if possible

Please see this link for an up-to-date comparison between the source branch of this pull request and the lworld branch.
As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

As you do not have Committer status in this project an existing Committer must agree to sponsor your change. Possible candidates are the reviewers of this PR (@coleenp, @Arraying) but any other Committer may sponsor as well.

➡️ To flag this PR as ready for integration with the above commit message, type /integrate in a new comment. (Afterwards, your sponsor types /sponsor in a new comment to perform the integration).

mlbridge · 2026-01-20T18:04:53Z

Webrevs

01: Full - Incremental (469258ee)
00: Full (61c1f4cf)

coleenp

I have a small request, but otherwise this looks great. Thank you for doing this!

coleenp · 2026-01-20T20:25:55Z

src/hotspot/share/interpreter/interpreterRuntime.cpp

+
+  // If the field is nullable and is marked null, return early.
+  if (LayoutKindHelper::is_nullable_flat(lk) &&
+      field_klass->is_payload_marked_as_null(cast_from_oop<address>(obj) + offset)) {


I wonder if there's a way to ask this and have the address calculation be hidden in InstanceKlass, like find_field_from_offset ? So we don't have to know here if the offset includes the header or not? Or even just add a is_payload_marked_as_null(oop, offset) to hide the calculation inside of inlineKlass.inline.hpp?

I am working on trying to create something for representing an InlineKlass payload in the runtime code. It achives exactly this. Current work in progress: 5df3317

Then I suggest we can hold off on this until Axel's patch is ready. I see he also takes care of similar casts + offsets in InlineKlass.cpp which is nice.

valhalla/src/hotspot/share/oops/inlineKlass.cpp

Lines 238 to 247 in a4fb7eb

if (is_payload_marked_as_null(cast_from_oop<address>(src) + offset)) {

return nullptr;

}

} // Fallthrough

case LayoutKind::BUFFERED:

case LayoutKind::NULL_FREE_ATOMIC_FLAT:

case LayoutKind::NULL_FREE_NON_ATOMIC_FLAT: {

Handle obj_h(THREAD, src);

oop res = allocate_instance(CHECK_NULL);

copy_payload_to_addr((void*)(cast_from_oop<address>(obj_h()) + offset), payload_addr(res), lk, false);

I don't know if it this should wait for Axel's patch, which I imagine is more extensive. I think you could push this first.

Arraying

Looks good, just one quick question.

Arraying · 2026-01-21T08:47:25Z

src/hotspot/cpu/aarch64/interp_masm_aarch64.cpp

-  bind(failed_alloc);
-  pop(obj);
-  bind(slow_path);
-  call_VM(obj, CAST_FROM_FN_PTR(address, InterpreterRuntime::read_flat_field),


In your writeup you mention call_VM_leaf is called, but I only see references to call_VM in the code yet the comments say call_VM_leaf. Do we/did we call_VM_leaf at any point?

The call to call_VM_leaf is in the copy-path:

MacroAssembler::flat_field_copy BarrierSetAssembler::flat_field_copy call_VM_leaf(CAST_FROM_FN_PTR(address, BarrierSetRuntime::value_copy), ...)

Makes sense, thank you for clarifying.

jsikstro · 2026-01-21T10:25:18Z

I removed the Handle from InterpreterRuntime::read_flat_field since we don't need it here because we aren't using the object after the call to read_payload_from_addr. There is a Handle inside read_payload_from_addr, just before allocating the buffered object, which is enough.

fparain · 2026-01-21T18:22:27Z

src/hotspot/cpu/x86/interp_masm_x86.cpp

-  load_unsigned_short(tmp2, Address(entry, in_bytes(ResolvedFieldEntry::field_index_offset())));
-
-  movptr(tmp1, Address(entry, ResolvedFieldEntry::field_holder_offset()));
-  get_inline_type_field_klass(tmp1, tmp2, field_klass);


This was the last place where get_inline_type_field_klass() was used, so this method can be removed from the MacroAssembler class.
Note that get_inline_type_field_klass() was not used in the aarch64 implementation of read_flat_field(), but the get_inline_type_field_klass() method still exists in the aarch64 MacroAssembler. This dead code could be removed too.

8375719: [lworld] Move reads of flat fields entirely to the runtime

61c1f4c

openjdk bot added the rfr Pull request is ready for review label Jan 20, 2026

coleenp approved these changes Jan 20, 2026

View reviewed changes

openjdk bot added the ready Pull request is ready to be integrated label Jan 20, 2026

Arraying approved these changes Jan 21, 2026

View reviewed changes

Dont handle:ize obj in InterpreterRuntime::read_flat_field

469258e

Arraying approved these changes Jan 21, 2026

View reviewed changes

fparain reviewed Jan 21, 2026

View reviewed changes

	if (is_payload_marked_as_null(cast_from_oop<address>(src) + offset)) {
	return nullptr;
	}
	} // Fallthrough
	case LayoutKind::BUFFERED:
	case LayoutKind::NULL_FREE_ATOMIC_FLAT:
	case LayoutKind::NULL_FREE_NON_ATOMIC_FLAT: {
	Handle obj_h(THREAD, src);
	oop res = allocate_instance(CHECK_NULL);
	copy_payload_to_addr((void*)(cast_from_oop<address>(obj_h()) + offset), payload_addr(res), lk, false);

8375719: [lworld] Move reads of flat fields entirely to the runtime #1936

Are you sure you want to change the base?

8375719: [lworld] Move reads of flat fields entirely to the runtime #1936

Conversation

jsikstro commented Jan 20, 2026 • edited by openjdk bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Progress

Issue

Reviewers

Reviewing

Uh oh!

bridgekeeper bot commented Jan 20, 2026

Uh oh!

openjdk bot commented Jan 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mlbridge bot commented Jan 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Webrevs

Uh oh!

coleenp left a comment

Choose a reason for hiding this comment

Uh oh!

coleenp Jan 20, 2026

Choose a reason for hiding this comment

Uh oh!

xmas92 Jan 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!

jsikstro Jan 21, 2026

Choose a reason for hiding this comment

Uh oh!

coleenp Jan 21, 2026

Choose a reason for hiding this comment

Uh oh!

Arraying left a comment

Choose a reason for hiding this comment

Uh oh!

Arraying Jan 21, 2026

Choose a reason for hiding this comment

Uh oh!

jsikstro Jan 21, 2026

Choose a reason for hiding this comment

Uh oh!

Arraying Jan 21, 2026

Choose a reason for hiding this comment

Uh oh!

jsikstro commented Jan 21, 2026

Uh oh!

fparain Jan 21, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

5 participants

jsikstro commented Jan 20, 2026 •

edited by openjdk bot

Loading

openjdk bot commented Jan 20, 2026 •

edited

Loading

mlbridge bot commented Jan 20, 2026 •

edited

Loading

xmas92 Jan 21, 2026 •

edited

Loading