RFR: 8275055: Improve HeapRegionRemSet::split_card()

[Stefan Johansson]

On Mon, 11 Oct 2021 15:11:12 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

Looks good, but I would like us to come up with better naming for the limit.

src/hotspot/share/gc/g1/heapRegionRemSet.cpp line 53:

> 51: 
> 52: void HeapRegionRemSet::initialize(HeapWord* heap_base_address) {
> 53:   _split_card_shift = MIN2((uint)HeapRegion::LogCardsPerRegion, per_card_region_card_limit()) + CardTable::card_shift;

This caught my eye a bit and the reason I think is that `per_card_region_card_limit()` sounds like a limit in actual cards, but it is in fact a "log" value. Could we call it something like `G1CardSetContainer::LogCardsPerRegionLimit` and access it directly?

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

Changes requested by sjohanss (Reviewer).

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