8322383: G1: Only preserve marks on objects that are actually moved #17159
Conversation
|
👋 Welcome back rkennke! A progress list of the required criteria for merging this PR into |
Webrevs
|
|
/label add hotspot-gc |
|
@albertnetymk |
| @@ -102,6 +103,7 @@ void G1FullGCCompactionPoint::forward(oop object, size_t size) { | |||
|
|
|||
| // Store a forwarding pointer if the object should be moved. | |||
| if (cast_from_oop<HeapWord*>(object) != _compaction_top) { | |||
| preserved_stack()->push_if_necessary(object, object->mark()); | |||
There was a problem hiding this comment.
Can this be made conditionally on whether the markword is NOT marked/forwarded? (IOW, move the added predicate in markWord here.) The rationale is to minimize changes to the shared code for G1 specific usages.
There was a problem hiding this comment.
My previous msg was probably unclear... I meant sth like:
if (!object->is_forwarded) {
preserved_stack()->push_if_necessary...
}(The newly added assert here is incorrect -- fails in GHA.)
There was a problem hiding this comment.
Oh I see! This makes more sense, yes. I made that change and cleaned up non G1 code.
BTW: I am working on new/alternative full-GCs for Serial, G1 and Shenandoah that don't store forwarding pointers in the mark-word at all, and thus avoid all that preserved-headers stuff (and together with the OM-world work that some Oracle engineers are working on, get rid of header displacement altogether). The SerialGC version is almost ready in Lilliput: openjdk/lilliput#122
|
@rkennke 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 246 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 |
|
Going to push as commit 16be388.
Your commit was automatically rebased without conflicts. |
openjdk
bot
removed
ready
The G1 full-GC preserves marks during marking, for all live objects in compaction region. However, not all live objects do actually move. In particular, the start of a compaction chain may have a sediment of all-live objects which would not move, and thus don't need to have their marks preserved.
The problem can easily be solved by preserving marks during forwarding. That also seems a more natural place to do that.
Testing:
Progress
Issue
Reviewers
Reviewing
Using
gitCheckout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/17159/head:pull/17159$ git checkout pull/17159Update a local copy of the PR:
$ git checkout pull/17159$ git pull https://git.openjdk.org/jdk.git pull/17159/headUsing Skara CLI tools
Checkout this PR locally:
$ git pr checkout 17159View PR using the GUI difftool:
$ git pr show -t 17159Using diff file
Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/17159.diff
Webrev
Link to Webrev Comment