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

Thomas Schatzl tschatzl at openjdk.java.net
Mon Nov 9 09:55:01 UTC 2020


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

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

Commit messages:
 - Initial import

Changes: https://git.openjdk.java.net/jdk/pull/1118/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=1118&range=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8256038
  Stats: 5 lines in 1 file changed: 1 ins; 2 del; 2 mod
  Patch: https://git.openjdk.java.net/jdk/pull/1118.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/1118/head:pull/1118

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


More information about the hotspot-gc-dev mailing list