8195148: Collapse G1SATBCardTableModRefBS and G1SATBCardTableLoggingModRefBS into a single G1BarrierSet

[Erik Österlund]

Hi Erik,

Thanks for the quick review. I will remove that newline before pushing.

/Erik

On 2018-02-27 15:30, Erik Helin wrote:
> 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