RFR: 8275055: Improve HeapRegionRemSet::split_card() [v3]

[Kim Barrett]

On Thu, 14 Oct 2021 08:41:15 GMT, Thomas Schatzl <tschatzl at openjdk.org> wrote:

>> Hi all,
>> 
>>   can I have reviews for this small change that improves HeapRegionRemSet::split_card() by reducing the number of (direct and indirect) memory accesses for it. It is hard to actually measure improvements because it's only called in concurrent code. So no particular improvements measured.
>> 
>> It also prepares that method for virtualizing the remembered set containers, allowing arbitrarily large heap region sizes [JDK-8275056](https://bugs.openjdk.java.net/browse/JDK-8275056). This change is required then because the splitting of a card index into "region" and "card within region" should not be dependent on *heap regions*.
>> 
>> Testing: gha, gc/g1 local testing.
>> 
>> Thanks,
>>   Thomas
>
> Thomas Schatzl has updated the pull request incrementally with one additional commit since the last revision:
> 
>   sjohanss review2

Changes requested by kbarrett (Reviewer).

src/hotspot/share/gc/g1/g1CardSetContainers.cpp line 30:

> 28: #include "utilities/globalDefinitions.hpp"
> 29: 
> 30: // The only limitation there is is from the G1CardSetArray.

s/there is is/is/

src/hotspot/share/gc/g1/g1CardSetContainers.hpp line 181:

> 179:   }
> 180: 
> 181:    // Log of largest card index that can be stored in any G1CardSetContainer

comment misindented

src/hotspot/share/gc/g1/heapRegionRemSet.inline.hpp line 65:

> 63: 
> 64: void HeapRegionRemSet::split_card(OopOrNarrowOopStar from, uint& card_region, uint& card_within_region) const {
> 65:   uintptr_t offset = pointer_delta(from, _heap_base_address, 1);

Why `uintptr_t` here?  `pointer_delta` returns `size_t`.  Changing this would also suggest changing the type of `_split_card_mask`.

src/hotspot/share/gc/g1/heapRegionRemSet.inline.hpp line 67:

> 65:   uintptr_t offset = pointer_delta(from, _heap_base_address, 1);
> 66:   card_region = offset >> _split_card_shift;
> 67:   card_within_region = (offset & _split_card_mask) >> CardTable::card_shift;

Maybe assert that the narrowing conversion is ok.

-------------

PR: https://git.openjdk.java.net/jdk/pull/5895