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

[Thomas Schatzl]

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)",

Thanks,
   Thomas