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

[sangheon.kim at oracle.com]

Hi Thomas,

On 11/26/19 1:04 AM, Thomas Schatzl wrote:
> Hi Sangheon,
>
>   thanks for looking at this.
>
> On 25.11.19 22:22, sangheon.kim at oracle.com wrote:
>> Hi Thomas,
>>
>> On 11/21/19 2:41 AM, 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,
> [...]>>>
>>>> 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/
>> Webrev.1 looks good in general.
>>
>> ========================
>> g1CollectionSet.cpp
>>   250   assert(old_rs_length <= new_rs_length,
>>   251          "Remembered set sizes must increase (changed from " 
>> SIZE_FORMAT " to " SIZE_FORMAT " region %u type %s)",
>>   252          old_rs_length, new_rs_length, hr->hrm_index(), 
>> hr->get_short_type_str());
>>   - I feel 'must increase' like 'old_rs_length < new_rs_length'. If 
>> you don't agree leave it as is. :)
>>
>> ========================
>> heapRegionRemSet.cpp
>>   200         Atomic::inc(&_num_occupied, memory_order_relaxed);
>> - I already asked to Thomas offline. He said Atomic operation is not 
>> necessary in this version but it is necessary for future patch when 
>> the lock is removed.
>
> http://cr.openjdk.java.net/~tschatzl/8233919/webrev.1_to_2/ (diff)
> http://cr.openjdk.java.net/~tschatzl/8233919/webrev.2/ (full)
>
> Fixes the comment by changing the comment text to:
>
>  251          "Remembered set decreased (changed from " SIZE_FORMAT " 
> to " SIZE_FORMAT " region %u type %s)",
>
Looks good.

Thanks,
Sangheon


> Thanks,
>   Thomas