RFR: 8202082: Remove explicit CMS checks in CardTableBarrierSetAssembler

[Erik Österlund]

Hi Aleksey,

Thanks for having a look at this patch.

You are right, the previous code has been inconsistent. Sometimes it 
checked just for CMS, and sometimes CMS + pre-cleaning. The actual 
property we are looking for is whether the card table is scanned 
concurrently or not (which I want to determine at a single place instead 
of all over the place). And for CMS, this is only when pre-cleaning is 
enabled. The callsites that only checked for CMS (and not CMS + 
pre-cleaning) are fencing despite there being no actual race to protect 
against.

Thanks,
/Erik

On 2018-04-23 10:37, Aleksey Shipilev wrote:
> On 04/20/2018 04:43 PM, Erik Österlund wrote:
>> In CardTableBarrierSetAssembler, we sometimes need fencing due to the card table being scanned
>> concurrently when using CMS with pre-cleaning. Rather than explicitly checking for those CMS flags
>> in the generic CardTableBarrierSetAssembler class, it is preferrable to check the CardTable
>> scanned_concurrently() property which already precisely reflects that.
>>
>> Bug:
>> https://bugs.openjdk.java.net/browse/JDK-8202082
>>
>> Webrev:
>> http://cr.openjdk.java.net/~eosterlund/8202082/webrev.00/
> This makes sense. Except that I see that scanned_concurrently is enabled only when
> CMSPrecleaningEnabled is true:
>
> CMSCardTable::CMSCardTable(MemRegion whole_heap) :
>      CardTableRS(whole_heap, CMSPrecleaningEnabled /* scanned_concurrently */) {
> }
>
> ...but old code inserts the barriers mostly with checking UseCMS only. I guess it does not matter
> much for CMS that does not have G1-like concurrent refinement threads, and we really do care about
> precleaning only?
>
> Otherwise looks good!
>
> -Aleksey
>