8195148: Collapse G1SATBCardTableModRefBS and G1SATBCardTableLoggingModRefBS into a single G1BarrierSet

Erik Helin erik.helin at oracle.com
Tue Feb 27 14:30:55 UTC 2018


On 02/26/2018 02:38 PM, Erik Österlund wrote:
> Hi,
> 
> G1 has two barrier sets: an abstract G1SATBCardTableModRefBS barrier set 
> that is incomplete and you can't use, and a concrete 
> G1SATBCardTableLoggingModRefBS barrier set is what is the one actually 
> used all over the place. The inheritance makes this code more difficult 
> to understand than it needs to be.
> 
> There should really not be an abstract G1 barrier set that is not used - 
> it serves no purpose. There should be a single G1BarrierSet instead 
> reflecting the actual G1 barriers used.
> 
> Webrev:
> http://cr.openjdk.java.net/~eosterlund/8195148/webrev.00/

Looks good, Reviewed. Just one very minor nit:

+G1BarrierSet::G1BarrierSet(
+  G1CardTable* card_table) :
+  CardTableModRefBS(card_table, 
BarrierSet::FakeRtti(BarrierSet::G1BarrierSet)),
+  _dcqs(JavaThread::dirty_card_queue_set())

Maybe put the constructor parameters on the same line as the constructor 
name (now that it fits), as in:

+G1BarrierSet::G1BarrierSet(G1CardTable* card_table) :
+  CardTableModRefBS(card_table, 
BarrierSet::FakeRtti(BarrierSet::G1BarrierSet)),
+  _dcqs(JavaThread::dirty_card_queue_set())

This looks a bit better IMO and seems to be a more commonly used style 
in HotSpot. No need for a new patch, just remember to fix this before 
pushing :)

Thanks,
Erik

> Bug:
> https://bugs.openjdk.java.net/browse/JDK-8195148
> 
> Thanks,
> /Erik


More information about the hotspot-dev mailing list