RFR (M): 8233919: Incrementally calculate the occupied cards in a heap region remembered set

[Stefan Johansson]

On 2019-11-21 11:41, Thomas Schatzl wrote:
> Hi,
> 
> On 20.11.19 11:42, Stefan Johansson wrote:
>> Hi Thomas,
>>
>> On 2019-11-12 16:24, Thomas Schatzl wrote:
>>> Hi all,
>>>
>>>    may I have reviews for this change that ultimately makes sure that 
>>> the number of occupied cards in a remembered set is only growing by 
>>> providing a per-OtherRegionsTable count that is atomically updated 
>>> when adding a remembered set entry.
>>>
>>> Note that this count may not be completely accurate due to races when 
>>> deleting a PerRegionTable (which is a known issue) from an 
>>> OtherRegionsTable; but that is no different than before.
>>>
>>> This helps improving the predictions in the young gen remset sampling 
>>> thread, and increase the performance of getting the occupancy count.
>>>
>>> Based on JDK-8233997, and JDK-8233998 also out for review.
>>>
>>> CR:
>>> https://bugs.openjdk.java.net/browse/JDK-8233919
>>> Webrev:
>>> http://cr.openjdk.java.net/~tschatzl/8233919/webrev/
>> I like this change and it looks good in general, just one small comment:
>> src/hotspot/share/gc/g1/heapRegionRemSet.cpp
>> ---
>>   247   bool added = prt->add_reference(from);
>>   248   Atomic::add(num_added_by_coarsening + (added ? 1 : 0), 
>> &_num_occupied, memory_order_relaxed);
>>
>> I would prefer:
>> if (prt->add_reference(from)) {
>>    num_added_by_coarsening++;
>> }
>> Atomic::add...
>>
>> I you disagree, leave it as is.
> 
> Fixed in
> http://cr.openjdk.java.net/~tschatzl/8233919/webrev.0_to_1/
> http://cr.openjdk.java.net/~tschatzl/8233919/webrev.1/
> 
Thanks Thomas!

Looks good,
Stefan
> Thanks for your review,
>    Thomas
> 
>