RFR: JDK-8199735: Mark word updates need to use Access API (original) (raw)

Roman Kennke rkennke at redhat.com
Mon Mar 26 16:39:40 UTC 2018

This looks good to me. Thank you for doing this.

Thanks Erik for reviewing!

The only thing I am a bit puzzled by is why the oopDesc::setmark member function takes a volatile markOop as parameter. That seems rather weird. But that is unrelated to your changes.

Yeah. What does that even mean in C++?

Roman

Thanks, /Erik On 2018-03-25 23:03, Roman Kennke wrote: Hi Erik,

Thanks for having a look.

A few comments:

1) Please make sure all files referencing these accessors that have been moved to the oop.inline.hpp file now #include the oop.inline.hpp file. I found that there are quite a few missing by looking for references to oopDesc::mark() with rtags. 2) I think the markaddr() function should now be called markaddrraw() for consistency. 3) I wonder if there should be raw() accessors that the GCs use instead. It feels like the GC should not go through the Access API to acquire mark words. You are of course right in all 3 points. Unfortunately, that causes massive ripples through the code base ;-) So here is what I did: I added raw() variants for stuff that is used by GC. I temporarily renamed all relevant methods, e.g. mark() -> markfluff() to make the compiler help me. Then for each occurance of any such call decided whether it should call the usual method, or the raw variant (as a general rule, everything under src/hotspot/share/gc uses the raw() variants). Also, I added #include "oop.inline.hpp" for every such occurance. Then I renamed all fluff() methods and calls back to usual. Did this exercise with and without PCH, with and without zero. Checked with grep if I might have missed some. Notes: bytecodeInterpreter.cpp needed some additional fixes to use cassetmark() instead of the Atomic::cmpxchg(). There are a bunch of methods in oop.hpp that are only used by GC, like islocked() etc. I haven't changed them to raw(). This passes tier1 tests release+fastdebug, and as mentioned above builds with and without PCH and zero. Differential: http://cr.openjdk.java.net/~rkennke/JDK-8199735/webrev.03.diff/ Full: http://cr.openjdk.java.net/~rkennke/JDK-8199735/webrev.03/ Better now? Best regards, Roman

Thanks, /Erik On 2018-03-21 20:21, Roman Kennke wrote: Am 19.03.2018 um 12:07 schrieb Roman Kennke: Am 19.03.2018 um 11:40 schrieb Roman Kennke: Currently, the mark word is accessed directly in oopDesc::mark() setmark() and a bunch of other accessors. Those need to use the Access API instead because GC might want to employ barriers on those accesses, pretty much like every other field in oopDesc.

Notice that this is not about accessing the bits and fields inside the markOop, but about accessing the header itself. http://cr.openjdk.java.net/~rkennke/JDK-8199735/webrev.01/ Testing: build fastdebug/release with and without PCH, passed tier1 fastdebug/release. Can I please get reviews? Thanks, Roman Just when I sent it, I realized that this is dropping the volatile from the mark word access. Fixed here:

http://cr.openjdk.java.net/~rkennke/JDK-8199735/webrev.02/ Ping?

-------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 836 bytes Desc: OpenPGP digital signature URL: <https://mail.openjdk.org/pipermail/hotspot-gc-dev/attachments/20180326/a8e491ad/signature.asc>

More information about the hotspot-gc-dev mailing list