-
Notifications
You must be signed in to change notification settings - Fork 5.4k
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
8282828: CDS uncompressed oops archive is not deterministic #8478
8282828: CDS uncompressed oops archive is not deterministic #8478
Conversation
👋 Welcome back iklam! A progress list of the required criteria for merging this PR into |
return hash; | ||
// Do not call p->identity_hash() as that will update the | ||
// object header. | ||
return primitive_hash(cast_from_oop<intptr_t>(p)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This fix doesn't affect the contents of the archive, but it's necessary to avoid incorrect output from -Xlog:cds+map=trace
.
Webrevs
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Change to compare.sh looks good. Thanks for fixing this!
/reviewers 2 |
@iklam 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:
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 no new commits pushed to the ➡️ To integrate this PR with the above commit message to the |
@erikj79 |
Looks good to me. But you need hotspot reviewers as well. And thanks for fixing this! I think this is the last major known non-reproducibility bug in the JDK! 🎉 /reviewers 3 |
@magicus |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
…uncompressed-oop-cds-archive-not-determinisic
Thanks @erikj79 @magicus @calvinccheung for the review. |
During
java -Xshare:dump -XX:-UseCompressedOops
, the location of the Java heap is chosen by the OS. Due to Address Space Layout Randomization, the heap will always start at a different location. This causes the archive for uncompressed oops ($JAVA_HOME/lib/server/classes_nocoops.jsa) to be non-deterministic.The fix is to patch the archived object pointers to make it look like the heap starts at a fixed address -- I chose 0x10000000, but the exact value doesn't really matter.
At runtime, the object pointers will be patched again according to the real location of the heap.
Tested with tiers 1-5. I am running builds-tier5 several times to test the xxx-cmp-baseline builds.
Progress
Issue
Reviewers
Reviewing
Using
git
Checkout this PR locally:
$ git fetch https://git.openjdk.java.net/jdk pull/8478/head:pull/8478
$ git checkout pull/8478
Update a local copy of the PR:
$ git checkout pull/8478
$ git pull https://git.openjdk.java.net/jdk pull/8478/head
Using Skara CLI tools
Checkout this PR locally:
$ git pr checkout 8478
View PR using the GUI difftool:
$ git pr show -t 8478
Using diff file
Download this PR as a diff file:
https://git.openjdk.java.net/jdk/pull/8478.diff