8379404: G1: Hide ConcurrentMarkThread reference from outside ConcurrentMark#30114
8379404: G1: Hide ConcurrentMarkThread reference from outside ConcurrentMark#30114tschatzl wants to merge 2 commits intoopenjdk:masterfrom
Conversation
Hi all, please review this cleanup that makes the `G1ConcurrentMarkThread` reference of `G1ConcurrentMark` completely private to the latter. Reason is that, similar to `G1ConcurrentRefine` this is an implementation detail, the caller is just interested in e.g. that all threads are stopped, not wanting to care about whether `G1ConcurrentMark` is actually initialized or how many threads it needs. The current code had some interesting circular reference too (`G1ConcurrentMark` calling `G1CollectedHeap´ which just went back calling `G1ConcurrentMark` to do actual work). Testing: gha Thanks, Thomas
|
👋 Welcome back tschatzl! A progress list of the required criteria for merging this PR into |
|
@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 3 new commits pushed to the
Please see this link for an up-to-date comparison between the source branch of this pull request and the ➡️ To integrate this PR with the above commit message to the |
Webrevs
|
stefank
left a comment
There was a problem hiding this comment.
This looks like a nice cleanup. One nit:
| friend class G1CMRemarkTask; | ||
| friend class G1CMRootRegionScanTask; | ||
| friend class G1CMTask; | ||
| friend class G1ClearBitMapTask; |
There was a problem hiding this comment.
It looks like this addition doesn't follow the sort order.
There was a problem hiding this comment.
Thomas explained that this is sorted case-sensitive and that we'll have a separate discussion about that outside of this PR.
|
Going to push as commit 7101a05.
Your commit was automatically rebased without conflicts. |
Hi all,
please review this cleanup that makes the
G1ConcurrentMarkThreadreference ofG1ConcurrentMarkcompletely private to the latter.Reason is that, similar to
G1ConcurrentRefinethis is an implementation detail, the caller is just interested in e.g. that all threads are stopped, not wanting to care about whetherG1ConcurrentMarkis actually initialized or how many threads it needs.The current code had some interesting circular reference too (
G1ConcurrentMarkcallingG1CollectedHeap´ which just went back callingG1ConcurrentMark` to do actual work).Testing: gha
Thanks,
Thomas
Progress
Issue
Reviewers
Reviewing
Using
gitCheckout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/30114/head:pull/30114$ git checkout pull/30114Update a local copy of the PR:
$ git checkout pull/30114$ git pull https://git.openjdk.org/jdk.git pull/30114/headUsing Skara CLI tools
Checkout this PR locally:
$ git pr checkout 30114View PR using the GUI difftool:
$ git pr show -t 30114Using diff file
Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/30114.diff
Using Webrev
Link to Webrev Comment