8264026: Remove dependency between free collection set and eagerly reclaim humongous object tasks #3154
Conversation
|
👋 Welcome back tschatzl! A progress list of the required criteria for merging this PR into |
| decrement_summary_bytes(cl.bytes_freed()); | ||
|
|
||
| phase_times()->record_fast_reclaim_humongous_time_ms((os::elapsedTime() - start_time) * 1000.0, | ||
| cl.humongous_objects_reclaimed()); | ||
| } | ||
|
|
||
| void G1CollectedHeap::rebuild_free_region_list() { | ||
| Ticks start = Ticks::now(); | ||
| _hrm.rebuild_free_list(workers()); |
There was a problem hiding this comment.
The first thing done inrebuild_free_list() is to drop the current free_list to rebuild if from scratch. So we should now be able to remove the call prepend_to_freelist(&local_cleanup_list) in eagerly_reclaim_humongous_regions(). Doing so the local_cleanup_list is only used for printing, and we should probably investigate if this can be done differently.
There was a problem hiding this comment.
There is JDK-8264027 filed for this. For now I was planning to keep this gathering of regions into the list but just factor out the printing code (as it's copy&pasted a bit), but we can certainly talk about more invasive measures.
The only way I see for now is that instead of gathering the elements in the list and printing in one go is printing immediately if we keep this functionality...
Thanks for your review.
There was a problem hiding this comment.
I agree, I think printing as we go sounds reasonable.
|
@tschatzl 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 50 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. ➡️ To integrate this PR with the above commit message to the |
|
/integrate |
openjdk
bot
added
integrated
|
@tschatzl Since your change was applied there have been 63 commits pushed to the
Your commit was automatically rebased without conflicts. Pushed as commit 3aee5ad. 💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored. |
Hi,
I am currently working on JDK-8214237 to merge some phases from
G1CollectedHeap::post_evacuate_collection_set()into less parallel phases.While investigating dependencies I found that
free_collection_setandeagerly_reclaim_humongous_regionshave a dependency:free_collection_setrebuilds the free list at the end (in another parallel phase :( ), andeagerly_reclaim_humongous_regionsadds to those regions.This dependency is unnecessary: just move the rebuilding of the free list after eagerly reclaiming humongous regions, and neither needs to care about updating the free regions list.
Testing: hs-tier1-5
Progress
Issue
Reviewers
Download
To checkout this PR locally:
$ git fetch https://git.openjdk.java.net/jdk pull/3154/head:pull/3154$ git checkout pull/3154To update a local copy of the PR:
$ git checkout pull/3154$ git pull https://git.openjdk.java.net/jdk pull/3154/head