8195148: Collapse G1SATBCardTableModRefBS and G1SATBCardTableLoggingModRefBS into a single G1BarrierSet

Kim Barrett kim.barrett at oracle.com
Mon Mar 5 22:02:59 UTC 2018


> 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.



More information about the hotspot-dev mailing list