-
Notifications
You must be signed in to change notification settings - Fork 6.2k
8330699: Obsolete -XX:+UseEmptySlotsInSupers #19720
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
Conversation
|
👋 Welcome back coleenp! A progress list of the required criteria for merging this PR into |
|
@coleenp 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 |
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.
Forgot to update special_jvm_flags, I think?
EDIT: Ah no, it should stay to fail cleanly, nevermind.
shipilev
left a comment
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.
Looks fine.
That's removed when the flag is removed in the next release. |
fparain
left a comment
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
Thank you for the removal of this code.
dholmes-ora
left a comment
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.
Looks good. But the flag table entry in arguments.cpp should be moved from the deprecated section to the obsolete section. Also the test
./test/hotspot/jtreg/runtime/CommandLine/VMDeprecatedOptions.java
needs updating.
Thanks.
|
There's no mention of this option in the VMDeprecatedOptions.java test. It was removed in: |
Sorry my bad. I was looking in an out-of-date branch |
dholmes-ora
left a comment
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.
Thanks for the update. Sorry for the delay.
|
@coleenp this pull request can not be integrated into git checkout supers
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 |
|
Thank you Aleksey, Fred and David. |
|
Going to push as commit 4b4a483.
Your commit was automatically rebased without conflicts. |
Remove code to not use empty slots in supers.
Tested with tier1-4.
Progress
Issue
Reviewers
Reviewing
Using
gitCheckout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/19720/head:pull/19720$ git checkout pull/19720Update a local copy of the PR:
$ git checkout pull/19720$ git pull https://git.openjdk.org/jdk.git pull/19720/headUsing Skara CLI tools
Checkout this PR locally:
$ git pr checkout 19720View PR using the GUI difftool:
$ git pr show -t 19720Using diff file
Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/19720.diff
Webrev
Link to Webrev Comment