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
8268390: G1 concurrent gc upgrade to full gc not working #4435
Conversation
|
@kstefanj This change now passes all automated pre-integration checks. 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 17 new commits pushed to the
As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.
|
Testing looks good, tier1 - tier3 + additional GC tests. |
Thanks @tschatzl and @kimbarrett. /integrate |
@kstefanj Since your change was applied there have been 28 commits pushed to the
Your commit was automatically rebased without conflicts. Pushed as commit f770f77. |
Please review this small change to enable concurrent GCs to be upgraded to Full collections when there are no free regions.
Summary
During the review of PR #4342 it was noted that we currently never upgrade concurrent GCs (from
VM_G1TryInitiateConcMark::doit()
) to full collections. Inshould_upgrade_to_full_gc()
there was a check to always return false for "concurrent" GC-causes. There is no obviously good reason for this, and this change will enable concurrent GCs to be upgraded.This could not be included in PR #4342 because that would make us hit the same problem as reported in JDK-8195158. The fix back then was to not upgrade concurrent GCs. The approach used now, which was added by PR #4357, is to use another GC cause for upgraded GCs.
What this change does is basically just removing the
should_do_concurrent_full_gc()
check fromshould_upgrade_to_full_gc()
and once that is donehas_regions_left_for_allocation()
might as well be folded intoshould_upgrade_to_full_gc()
Testing
Manual testing to verify this does not cause the assertion failure from JDK-8195158. Currently running Mach5 testing.
Progress
Issue
Reviewers
Reviewing
Using
git
Checkout this PR locally:
$ git fetch https://git.openjdk.java.net/jdk pull/4435/head:pull/4435
$ git checkout pull/4435
Update a local copy of the PR:
$ git checkout pull/4435
$ git pull https://git.openjdk.java.net/jdk pull/4435/head
Using Skara CLI tools
Checkout this PR locally:
$ git pr checkout 4435
View PR using the GUI difftool:
$ git pr show -t 4435
Using diff file
Download this PR as a diff file:
https://git.openjdk.java.net/jdk/pull/4435.diff