8371199: [lworld] Flattening of nullable elements of classes similar to j.l.Long

[merykitty]

Hi,

Currently, the layout of a nullable j.l.Long, if flattened, must be at least 65 bits. This exceeds the maximum size a GPR can generally hold, as well as the default object alignment of Hotspot. As a result, accessing such elements atomically require 128-bit atomic accesses, as well as mechanism from the GC to allocate overaligned objects. And even then, we will encounter the same issue, just with larger objects.

We can observe that, a nullable element does not really have an additional bit of information, it just has an additional state (e.g. a nullable j.l.Long has 2^64 + 1 states, not 2^65), and it is just unfortunate that we need a whole bit to be able to represent such an element. However, we can rely on the fact that all payload bits are irrelevant when the null marker is 0 to make a sequence of more than 1 memory access instructions look atomic.

C1 has not been implemented yet, we will bailout the compilation when encountering such an access. I will implement this functionality later.

Please take a look and leave your reviews, thanks a lot.

---

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-8371199: [lworld] Flattening of nullable elements of classes similar to j.l.Long (Enhancement - P4)

Reviewing

Using git

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

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

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 1720

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

Using diff file

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

Using Webrev

Link to Webrev Comment

[bridgekeeper]

👋 Welcome back qamai! 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]

@merykitty This change is no longer ready for integration - check the PR body for details.

[mlbridge]

Webrevs

• 03: Full - Incremental (217c3670)
• 02: Full - Incremental (e80107b7)
• 01: Full - Incremental (542c2fd8)
• 00: Full (de358f1a)

[forax]

Hello,
I do not fully understand why you care about GCs given that the 64bits payload of the value type can not be a pointer or contains a pointer ?
I'm sure i'm missing something :)

Also I suppose that at some point this patch (or a following one) will change the VarHandle implementation (or Unsafe ?), because currently the read/write through a VarHandle will not have the required the memory barriers.

[merykitty]

@forax Suppose a value class like this value record(String name, int idx), then it should contain a pointer in its payload.

For your second point, Unsafe::putFlatValue and Unsafe::getFlatValue will call into InlineKlass::write_value_to_addr and InlineKlass::read_payload_from_addr, respectively, which are covered by this patch. j.l.i.VarHandleNonAtomicFlatValues, in turns, calls Unsafe::getFlatValue for the get method and Unsafe::putFlatValue for the set method. So, I believe they are also fine.

[SirYwell]

Suppose a value class like this value record(String name, int idx), then it should contain a pointer in its payload.

I don't know how this is currently implemented in the GC, but wouldn't it make sense for the GC to check whether the value is marked as null first, and ignore the rest of the value in that case?

Especially for ZGC flattening currently doesn't happen even for value record(String name) from my understanding (and more generally, with compressed oops disabled?), this might be beneficial. I don't know how this interacts with GC barriers, there is most likely more to it than I know of :)

That said, restricting to payloads without oops makes this change easier to reason about, so - assuming it makes sense - extending to payloads with oops as a follow-up might be better.

[dansmithcode]

FYI, we explored some ideas along these lines a couple of years ago. Ended up concluding that it was a dead end—I'd suggest discussing on [email protected] if you'd like to understand what the concerns were.

[forax]

@forax Suppose a value class like this value record(String name, int idx), then it should contain a pointer in its payload.

yes, i get that.
Let me try to rephrase, what your are proposing in optimization that only work if the payload does not contains a pointer.
The pointer can be a 32bits or a 64 bits pointer, it makes no difference. So i've trouble to understand the code around
https://github.com/openjdk/valhalla/pull/1720/files#diff-b0ea998b1ae6491c87dae54a2a70644f8e957e7f3522f780883d72fb29107aeaR1845
that behave differently depending on the GC.

For your second point, Unsafe::putFlatValue and Unsafe::getFlatValue will call into InlineKlass::write_value_to_addr and InlineKlass::read_payload_from_addr, respectively, which are covered by this patch. j.l.i.VarHandleNonAtomicFlatValues, in turns, calls Unsafe::getFlatValue for the get method and Unsafe::putFlatValue for the set method. So, I believe they are also fine.

Okay, so i suppose it means that because this considered as non-atomic on the VarHandle side, something like a CAS operation is not supported.

[merykitty]

@forax That line is in the else branch, i.e. the branch in which we access the payload as a whole. The only change to that part in this PR is to indent it 2 spaces further.

Okay, so i suppose it means that because this considered as non-atomic on the VarHandle side, something like a CAS operation is not supported.

Yes, I don't think we support CAS operations for flat elements in general.

[TobiHartmann]

Ended up concluding that it was a dead end—I'd suggest discussing on [email protected] if you'd like to understand what the concerns were.

FTR, @forax provided an example here: https://mail.openjdk.org/pipermail/valhalla-dev/2025-November/016074.html

The issue of resurrecting an old value generally prevents us from flattening 64 bit + null marker, unfortunately.

[TobiHartmann]

@SirYwell @forax I think there's some confusion around the GC related code here. First of all, this is not specific to @merykitty's changes. This is not new, he just moved existing code.

That particular code is needed to support 64-bit flat accesses with one or two 32-bit compressed oops. Currently, two oops are only possible if the field/array is null-free. Different GCs need different barriers, so we need to emit different code depending on which GC is used.