RFR: 8256038: G1: Improve comment about mark word handling of displaced mark words

Stefan Johansson sjohanss at openjdk.java.net
Mon Nov 9 13:48:56 UTC 2020


On Mon, 9 Nov 2020 09:48:48 GMT, Thomas Schatzl <tschatzl at openjdk.org> wrote:

> Hi all,
> 
>   can I have reviews for this small comment change that fixes an imho completely misleading comment into something more understandable. Reasons outlined below:
> 
> I.e. in the code:
> 
>        if (old_mark.has_displaced_mark_helper()) {
>         // In this case, we have to install the mark word first,
>         // otherwise obj looks to be forwarded (the old mark word,
>         // which contains the forward pointer, was copied)
>         obj->set_mark(old_mark);
>         markWord new_mark = old_mark.displaced_mark_helper().set_age(age);
>         old_mark.set_displaced_mark_helper(new_mark);
> 
> "in this case ... the obj looks to be forwarded" - it is not true that only in this case the mark word looks to be forwarded because of the copy. G1 always copies the mark word containing the forwarded pointer, i.e. after the copy, the mark word in obj is always the forwarding pointer.
> That's why we need to set it to the (eventually updated) old mark word value in all cases....
> 
> "we have to install the mark word first" - the order of installing the mark word and updating the displaced mark word is completely irrelevant here - the point is that we need to update the age in the displaced mark word and must not change the old mark word in this branch. The obj->set_mark() call can be at any position actually. 
> 
> I went with fixing the comment and setting the mark word last in that code block to be similar to other cases. I refrained from other refactorings like refactoring this into (inlined) methods.
> 
> Testing: compilation, some quick tests like gcbasher (but errors here typically make building the image fail).
> 
> Thanks,
>   Thomas

Looks good.

-------------

Marked as reviewed by sjohanss (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/1118



More information about the hotspot-gc-dev mailing list