8305898: Alternative self-forwarding mechanism

[rkennke]

Currently, the Serial, Parallel and G1 GCs store a pointer to self into object headers to indicate promotion failure. This is problematic for compact object headers (JDK-8294992) because it would (temporarily) over-write the crucial class information, which we need for heap parsing. I would like to propose an alternative: use the currently unused 3rd header bit (previously biased-locking bit) to indicate that an object is 'self-forwarded'. That preserves the crucial class information in the upper bits of the header until the full header gets restored.

This is a trimmed-down/simplified version of the original proposal #13779:

• It doesn't use/introduce any flags and avoids the associated branching.
• It doesn't (need to) deal with displaced headers. (Current code would preserve header if necessary, Lilliput code would not use displaced headers and set the 3rd bit directly in existing header.)

Testing:

• hotspot_gc
• tier1
• tier2

---

Progress

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

Integration blocker

 ⚠️ Whitespace errors (failed with updated jcheck configuration in pull request)

Issue

• JDK-8305898: Alternative self-forwarding mechanism (Enhancement - P4)

Reviewers

• Thomas Schatzl (@tschatzl - Reviewer)
• Albert Mingkun Yang (@albertnetymk - Reviewer)

Reviewing

Using git

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

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

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 17755

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

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/17755.diff

Webrev

Link to Webrev Comment

[bridgekeeper]

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

[openjdk]

@rkennke The following label will be automatically applied to this pull request:

• hotspot

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing list. If you would like to change these labels, use the /label pull request command.

[mlbridge]

Webrevs

• 03: Full - Incremental (f844954f)
• 02: Full - Incremental (fbfb1e73)
• 01: Full - Incremental (c7ddf821)
• 00: Full (aa5e9876)

[rkennke]

/label add hotspot-gc

[openjdk]

@rkennke
The hotspot-gc label was successfully added.

[stefank]

This adds a branch to oopDesc::forwardee. Do you have an GC time measurements to show that this isn't causing a regression?

[rkennke]

This adds a branch to oopDesc::forwardee. Do you have an GC time measurements to show that this isn't causing a regression?

I just ran the (young-GC-heavy) derby benchmark, with more iterations, which does about 1600 GC cycles. Those are the average GC times of young GCs, running with G1:
baseline: 2.88624ms
selffwd: 2.88000ms

With Parallel GC, I get:
baseline: 2.80709
selffwd: 2.74724

When repeating the experiment I get similar results, sometimes baseline is slightliy better, sometimes selffwd is slightly better, but I don't see anything that would support a 'regression' with selffwd.

[albertnetymk]

Could you explain why this is required? (I'd expect oopDesc to be properly aligned already, since it's >= 8 bytes.)

[rkennke]

This is a problem with x86_32 builds, where oopDesc is only 4 byte. It is only a problem in this test, in real world, objects in the heap are always at least 8-byte-aligned.

[albertnetymk]

Even on 32bits system, shouldn't _mark = 4 bytes and _metadata = 4 bytes, so the total size of oopDesc is 8 bytes?

[xmas92]

The alignment of a struct must just ensure that the alignments requirement of its fields are satisfied (in the general case) So a struct of two 4 byte ints will have an alignof 4 bytes and a sizeof 8 bytes

#include <cstdint>
struct S {
    uint32_t a;
    uint32_t b;
};

static_assert(alignof(S) == 4);
static_assert(sizeof(S) == 8);

[albertnetymk]

Thank you for the explanation; I mistakenly thought a struct is aligned to its size...

[tschatzl]

Please close out PR#13779.

[openjdk]

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

[rkennke]

A potential improvement to this might be that we could avoid preserving the header around self-forwarding/promo-failure (and therefore remove a bunch of code/complexity). What's needed for this is a fast way to detect is_forwarded() that includes both scenarios normal-forwarded and self-forwarded.

Currently I also set normal-forwarded bits when self-forwarding to facilitate simple (v & 3) == 3 test. We would need something like ((v & 4) != 0) || ((v & 3) == 3) test, but ideally without the branch. We could test ((v & 7) > 2. That is same number of ops as the current test, so wouldn't add any overhead.

Then, instead of init-ing the self-forwarded header, and restoring any 'interesting' marks (hashed, locked), we could simply flip-back the self-fwd bit and that'd be it, and would not need any header-preservation in the promo-failure path.

1. Do we want this?
2. Do we want this in this PR, or better in a follow-up PR (including the removal of header-preservation around promo-failures)?

[albertnetymk]

I'm leaning towards doing that in a follow-up PR.

[tschatzl]

I'm leaning towards doing that in a follow-up PR.

+1

[rkennke]

I'm leaning towards doing that in a follow-up PR.

Ok. But maybe this would be an incentive to do this change ahead of the Lilliput main JEP, in JDK23? This PR, as it is proposed now doesn't provide much value in itself, but with the proposed follow-up, it does improve/simplify promotion-failure handling, and restricts usage of the PreservedMarks stuff to the full-GCs only.

[tschatzl]

We often do preparatory changes separately from the main improvement for several reasons, one of them to reduce complexity of the follow-up.

[openjdk]

@rkennke this pull request can not be integrated into master due to one or more merge conflicts. To resolve these merge conflicts and update this pull request you can run the following commands in the local repository for your personal fork:

git checkout JDK-8305898-v2
git fetch https://git.openjdk.org/jdk.git master
git merge FETCH_HEAD
# resolve conflicts and follow the instructions given by git merge
git commit -m "Merge master"
git push

[rkennke]

Will re-open when ready in Lilliput