RFR: 8067499: G1SATBCardTableModRefBS should not inherit from CardTableModRefBSForCTRS

Bengt Rutisson bengt.rutisson at oracle.com
Wed Dec 17 12:53:43 UTC 2014

Hi Kim,

On 2014-12-17 07:14, Kim Barrett wrote:
> Please review this change to G1SATBCardTableModRefBS to derive from
> CardTableModRefBS rather than CardTableModRefBSForCTRS.
> Note: I will need a sponsor for this change.
> That inheritance change runs afoul of SharedHeap providing remembered set
> support.  That seems inappropriate, since the provided remembered set support
> is oriented toward generational collection (with incremental update barriers),
> but generations show up in GenCollectedHeap, which is derived from SharedHeap.
> To address that, the rem_set() member (and associated data member) were moved
> from SharedHeap down to GenCollectedHeap.  This permitted some cleanup in the
> G1 code, eliminating references to the otherwise vestigal global remembered
> set object.  (G1 remembered sets are a different sort of creature.)
> A different way of addressing the remembered set location would have been to
> change G1CollectedHeap to derive from CollectedHeap rather than SharedHeap,
> and that's a change that has been suggested as a technical debt cleanup.  That
> change would have required similar changes to G1 code, plus a number of other
> changes.  This move of remembered set support can be viewed as a step toward
> changing the heap hierarchy.
> It may be that the G1 barrier set shouldn't derive from CardTableModRefBS
> either; CardTableModRefBS provides a lot of stuff that G1 doesn't use or need,
> some of which has to be overridden or called for compatibility.
> Alternatively, it might be that some things presently in CardTableModRefBS
> should be moved down to CardTableModRefBSForCTRS.  But again, that larger
> refactoring would require more changes, and this move of remembered set
> support and shift of the G1 barrier set base class can be viewed as a step
> toward that larger refactoring.
> CR:
> https://bugs.openjdk.java.net/browse/JDK-8067499
> Webrev:
> http://cr.openjdk.java.net/~kbarrett/8067499/webrev

Great cleanup!

One thought about this code in cardTableRS.cpp:

   42   guarantee(!UseG1GC, "sanity");
   43 #endif

I don't think we need the INCLUDE_ALL_GCS guard here since the UseG1GC 
is always available. Also, I think I would prefer to check for what we 
actually expect instead of what we don't expect. This is what we are 
expecting, right?

guarantee(UseSerialGC | UseConcMarkSweepGC, "santiy");


> Testing:
> Local jtreg of hotspot/test/{runtime,gc}.
> refworkload with G1, CMS, and ParOld collectors.

More information about the hotspot-gc-dev mailing list