8195148: Collapse G1SATBCardTableModRefBS and G1SATBCardTableLoggingModRefBS into a single G1BarrierSet

Erik Österlund erik.osterlund at oracle.com
Mon Mar 5 22:11:09 UTC 2018

Hi Kim,

On 2018-03-05 23:02, Kim Barrett wrote:
>> On Mar 5, 2018, at 4:10 PM, Erik Österlund <erik.osterlund at oracle.com> wrote:
>> Hi Kim,
>> On 2018-03-05 20:33, Kim Barrett wrote:
>>>> On Mar 5, 2018, at 5:08 AM, Erik Österlund <erik.osterlund at oracle.com> wrote:
>>>> Hi Kim,
>>>> New full webrev:
>>>> http://cr.openjdk.java.net/~eosterlund/8195148/webrev.01/
>>>> Incremental:
>>>> http://cr.openjdk.java.net/~eosterlund/8195148/webrev.00_01/
>>> src/hotspot/share/gc/g1/g1BarrierSet.inline.hpp
>>> 40   if (oopDesc::is_null(heap_oop)) {
>>> Test is backward.
>> Webrev: http://cr.openjdk.java.net/~eosterlund/8195148/webrev.02/
>> Incremental: http://cr.openjdk.java.net/~eosterlund/8195148/webrev.01_02/
>> Fixed.
> Good.
>>> I’m not sure yet what the new include changes in other files are about.
>> It breaks an include cycle that is necessary for ZGC (and probably Shenandoah too) to build with #include "oops/oop.inline.hpp" being in g1BarrierSet.inline.hpp. It reproduces when files include both barrierSet.inline.hpp and access.inline.hpp. Because barrierSet.inline.hpp includes barrierSetConfig.inline.hpp which includes all concrete barrier sets, which now also includes oop.inline.hpp which includes access.inline.hpp which includes barrierSet.inline.hpp again. This cycle causes resulution to barrierset accessors to happen before their metafunctions have been declared that translate barrierset types to/from enum values, which breaks the build. This bad cycle is broken with these changes by having access.inline.hpp include barrierSetConfig.inline.hpp instead of barrierSet.inline.hpp so that the barrierset type translation metafunctions have always been declared when resulution is defined.
> I suspected it was something like that.  Can you provide more detail,
> or much better, tell me how to reproduce the problem?  In the absence
> of such, the proposed changes look kind of ad hoc and fragile to me.
> I've been thinking about some questions about how we write and use
> .inline.hpp files, and have some ideas that may or may not be
> relevant.  If I could reproduce the problem at hand, I might be able
> to suggest some alternative ideas.  Depending on what I find, I might
> say go ahead with the proposed change, but I'd like to have a look
> first, in case there's a (relatively) simple alternative that seems
> more solid.

The easiest way to reproduce is to either:
1) Add #include "oops/oop.inline.hpp" in 
g1SATBCardTableModRefBS.inline.hpp in the Z repo, or:
2) Add any minimal no-op barrier set to jdk-hs with a name that comes 
after "g1" in lexicographical order (and hence include order), and plug 
it in to the barrierSetConfig files, and then add #include 
"oops/oop.inline.hpp" to the g1SATBCardTableModRefBS.inline.hpp file.


More information about the hotspot-dev mailing list