RFR (S): 8242036: G1 HeapRegionRemSet::_n_coarse_entries could be a bool

[Albert Yang]

I don't think calling `at_put` unconditionally is clearer, since 
`_coarse_map.at_put` needs be after `_coarse_map.reinitialize`, which 
only happens when `_has_coarse_entries` is false.

Maybe I misunderstood what you meant.

On 2020-07-24 10:23, Kim Barrett wrote:
>> On Jul 23, 2020, at 8:29 AM, Albert Yang <albert.m.yang at oracle.com> wrote:
>>
>> I misunderstood the original request from the internal discussion.
>>
>> The variable is marked `volatile`, and access to it uses `Atomic::load/store`, or `Atomic::load_acquire/release_store` only when memory ordering is needed.
>>
>> https://bugs.openjdk.java.net/browse/JDK-8242036
>>
>> the complete patch:
>>
>> http://cr.openjdk.java.net/~lkorinth/albert/8242036/2/
>>
>> Tested: hotspot-gc
>>
>> PS: My apologies for the inconvenience.
>>
>> /Albert
> src/hotspot/share/gc/g1/heapRegionRemSet.cpp
> 267     if (!_coarse_map.at(max_hrm_index)) {
> 268       _coarse_map.at_put(max_hrm_index, true);
> 269     }
>
> The old code conditionalized on !_course_map.at() to avoid
> incrementing _n_coarse_entries if the entry was already set.  That's
> no longer needed.  It seems like it might be clearer to just do the
> at_put unconditionally.  If so, also change the comment on line 264 to
> say something like "Ensure the corresponding coarse bit is set."
>
> src/hotspot/share/gc/g1/heapRegionRemSet.cpp
> 273     assert(_coarse_map.at(max_hrm_index) == false, "No coarse entries");
>
> +1 to Thomas's suggestion to not compare to false.
>
> Sorry I didn't notice either of these previously.
>