Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

8305895: Implement JEP 450: Compact Object Headers (Experimental) #20640

Closed
wants to merge 13 commits into from

Conversation

rkennke
Copy link
Contributor

@rkennke rkennke commented Aug 20, 2024

This is the main body of the JEP 450: Compact Object Headers (Experimental).

Main changes:

  • Introduction of the (experimental) flag UseCompactObjectHeaders. All changes in this PR are protected by this flag. The purpose of the flag is to provide a fallback, in case that users unexpectedly observe problems with the new implementation. The intention is that this flag will remain experimental and opt-in for at least one release, then make it on-by-default and diagnostic (?), and eventually deprecate and obsolete it. However, there are a few unknowns in that plan, specifically, we may want to further improve compact headers to 4 bytes, we are planning to enhance the Klass* encoding to support virtually unlimited number of Klasses, at which point we could also obsolete UseCompressedClassPointers.
  • The compressed Klass* can now be stored in the mark-word of objects. In order to be able to do this, we are building on 8305898: Alternative self-forwarding mechanism #20603 and 8305896: Alternative full GC forwarding #20605 to protect the relevant (upper 32) bits of the mark-word. Significant parts of this PR deal with loading the compressed Klass* from the mark-word. This PR also changes some code paths (mostly in GCs) to be more careful when accessing Klass* (or mark-word or size) to be able to fetch it from the forwardee in case the object is forwarded.
  • The identity hash-code is temporarily narrowed to 25 bits. As soon as we get Tiny Class-Pointers (planned before the JEP can be integrated, and to be opened for review soon), we will widen the hash-bits back to 31 bits.
  • Instances can now have their base-offset (the offset where the field layouter starts to place fields) at offset 8 (instead of 12 or 16).
  • Arrays will can now store their length at offset 8.
  • CDS can now write and read archives with the compressed header. However, it is not possible to read an archive that has been written with an opposite setting of UseCompactObjectHeaders. Some build machinery is added so that _coh variants of CDS archives are generated, next to the _nocoops variant.
  • Note that oopDesc::klass_offset_in_bytes() is not used by +UCOH paths anymore. The only exception is C2, which uses it as a placeholder/identifier of the special memory slice that only LoadNKlass uses. The backend then extracts the original oop and loads its mark-word and extracts the narrow-Klass* from that. I played with other approaches to implement LoadNKlass. Expanding it as a macro did not easily work, because C2 is missing a way to cast a word-sized integral to a narrow-Klass* (or at least I could not find it), and also I fear that doing so could mess with optimizations. This may be useful to revisit. OTOH, the approach that I have taken works and is similar to DecodeNKlass and similar instructions.

Testing:
(+UseCompactObjectHeaders tests are run with the flag hard-patched into the build, to also catch @flagless tests.)
The below testing has been run many times, but not with this exact base version of the JDK. I want to hold off the full testing until we also have the Tiny Class-Pointers PR lined-up, and test with that.

  • tier1 (x86_64)
  • tier2 (x86_64)
  • tier3 (x86_64)
  • tier4 (x86_64)
  • tier1 (aarch64)
  • tier2 (aarch64)
  • tier3 (aarch64)
  • tier4 (aarch64)
  • tier1 (x86_64) +UseCompactObjectHeaders
  • tier2 (x86_64) +UseCompactObjectHeaders
  • tier3 (x86_64) +UseCompactObjectHeaders
  • tier4 (x86_64) +UseCompactObjectHeaders
  • tier1 (aarch64) +UseCompactObjectHeaders
  • tier2 (aarch64) +UseCompactObjectHeaders
  • tier3 (aarch64) +UseCompactObjectHeaders
  • tier4 (aarch64) +UseCompactObjectHeaders
  • Running as a backport in production since >1 year.

Progress

  • Change must not contain extraneous whitespace
  • Commit message must refer to an issue
  • Change requires a JEP request to be targeted
  • Change requires CSR request JDK-8306000 to be approved
  • Change must be properly reviewed (3 reviews required, with at least 1 Reviewer, 2 Authors)

Issues

  • JDK-8305895: Implement JEP 450: Compact Object Headers (Experimental) (Enhancement - P4)
  • JDK-8294992: JEP 450: Compact Object Headers (Experimental) (JEP)
  • JDK-8306000: Add experimental -XX:+UseCompactObjectHeaders flag (CSR)

Reviewers

Reviewing

Using git

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

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

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 20640

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

Using diff file

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

Webrev

Link to Webrev Comment

@bridgekeeper
Copy link

bridgekeeper bot commented Aug 20, 2024

👋 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
Copy link

openjdk bot commented Aug 20, 2024

❗ This change is not yet ready to be integrated.
See the Progress checklist in the description for automated requirements.

@openjdk openjdk bot added the csr Pull request needs approved CSR before integration label Aug 20, 2024
@openjdk
Copy link

openjdk bot commented Aug 20, 2024

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

  • build
  • core-libs
  • graal
  • hotspot
  • serviceability
  • shenandoah

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

@openjdk openjdk bot added graal graal-dev@openjdk.org serviceability serviceability-dev@openjdk.org build build-dev@openjdk.org hotspot hotspot-dev@openjdk.org core-libs core-libs-dev@openjdk.org shenandoah shenandoah-dev@openjdk.org labels Aug 20, 2024
@AlanBateman
Copy link
Contributor

/label remove core-libs

@openjdk openjdk bot removed the core-libs core-libs-dev@openjdk.org label Aug 20, 2024
@openjdk
Copy link

openjdk bot commented Aug 20, 2024

@AlanBateman
The core-libs label was successfully removed.

@rkennke rkennke force-pushed the JDK-8305895-v3 branch 2 times, most recently from 35b88fe to 18b5363 Compare August 21, 2024 10:43
@rkennke
Copy link
Contributor Author

rkennke commented Aug 21, 2024

/label remove graal

@rkennke
Copy link
Contributor Author

rkennke commented Aug 21, 2024

/label remove shenandoah

@rkennke
Copy link
Contributor Author

rkennke commented Aug 21, 2024

/label add hotspot-runtime
/label add hotspot-gc

@openjdk openjdk bot removed the graal graal-dev@openjdk.org label Aug 21, 2024
@openjdk
Copy link

openjdk bot commented Aug 21, 2024

@rkennke
The graal label was successfully removed.

@openjdk openjdk bot removed the shenandoah shenandoah-dev@openjdk.org label Aug 21, 2024
@openjdk
Copy link

openjdk bot commented Aug 21, 2024

@rkennke
The shenandoah label was successfully removed.

@openjdk openjdk bot added the hotspot-runtime hotspot-runtime-dev@openjdk.org label Aug 21, 2024
@openjdk
Copy link

openjdk bot commented Aug 21, 2024

@rkennke
The hotspot-runtime label was successfully added.

@openjdk openjdk bot added the hotspot-gc hotspot-gc-dev@openjdk.org label Aug 21, 2024
@openjdk
Copy link

openjdk bot commented Aug 21, 2024

@rkennke
The hotspot-gc label was successfully added.

@rkennke rkennke changed the base branch from master to pr/20605 August 21, 2024 11:40
@openjdk
Copy link

openjdk bot commented Aug 21, 2024

@rkennke
This pull request will not be integrated until the JEP-450 has been targeted.

@openjdk openjdk bot added the jep label Aug 21, 2024
Comment on lines +2574 to +2575
lea(dst, Address(obj, index, Address::lsl(scale)));
ldr(dst, Address(dst, offset));
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It ignores the offset, right? Or are you saying that offset must be 0 on that path?

@rkennke rkennke changed the title 8305895: Implementation: JEP 450: Compact Object Headers (Experimental) Implement JEP 450: Compact Object Headers (Experimental) Aug 22, 2024
@openjdk openjdk bot removed the rfr Pull request is ready for review label Aug 22, 2024
Copy link
Member

@magicus magicus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Build changes look good. I have not looked at any other changes.

/reviewers 3

@openjdk
Copy link

openjdk bot commented Aug 22, 2024

@magicus
The total number of required reviews for this PR (including the jcheck configuration and the last /reviewers command) is now set to 3 (with at least 1 Reviewer, 2 Authors).

@magicus
Copy link
Member

magicus commented Aug 22, 2024

@rkennke Note that the Skara bot removed the RFR label when you changed the title to no longer match a JBS issue. This means that no emails will be sent to the corresponding lists. I am not sure if this was intentional on your part.

@rkennke rkennke changed the title Implement JEP 450: Compact Object Headers (Experimental) 8305895: Implement JEP 450: Compact Object Headers (Experimental) Aug 22, 2024
@rkennke
Copy link
Contributor Author

rkennke commented Aug 22, 2024

@rkennke Note that the Skara bot removed the RFR label when you changed the title to no longer match a JBS issue. This means that no emails will be sent to the corresponding lists. I am not sure if this was intentional on your part.

Thanks for pointing that out! No it was not intentional. Mark changed the title in the JBS issue, and I copied that over, but forgot the actual issue number. Should be fixed now.

@openjdk openjdk bot added the rfr Pull request is ready for review label Aug 22, 2024
@rkennke rkennke changed the base branch from pr/20605 to master August 22, 2024 11:48
@openjdk
Copy link

openjdk bot commented Aug 22, 2024

@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-8305895-v3
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

@openjdk
Copy link

openjdk bot commented Aug 22, 2024

⚠️ @rkennke This pull request contains merges that bring in commits not present in the target repository. Since this is not a "merge style" pull request, these changes will be squashed when this pull request in integrated. If this is your intention, then please ignore this message. If you want to preserve the commit structure, you must change the title of this pull request to Merge <project>:<branch> where <project> is the name of another project in the OpenJDK organization (for example Merge jdk:master).

@openjdk openjdk bot added the merge-conflict Pull request has merge conflict with target branch label Aug 22, 2024
@rkennke
Copy link
Contributor Author

rkennke commented Aug 22, 2024

Superseding by #20677

@rkennke rkennke closed this Aug 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build build-dev@openjdk.org csr Pull request needs approved CSR before integration hotspot hotspot-dev@openjdk.org hotspot-gc hotspot-gc-dev@openjdk.org hotspot-runtime hotspot-runtime-dev@openjdk.org jep merge-conflict Pull request has merge conflict with target branch rfr Pull request is ready for review serviceability serviceability-dev@openjdk.org
Development

Successfully merging this pull request may close these issues.

5 participants