RFR: 8252041: G1: Fix incorrect uses of HeapRegionManager::max_length [v4]
Thomas Schatzl
thomas.schatzl at oracle.com
Thu Sep 17 18:24:55 UTC 2020
Hi Kim,
On 17.09.20 13:12, Kim Barrett wrote:
>> On Sep 17, 2020, at 3:46 AM, Thomas Schatzl <tschatzl at openjdk.java.net> wrote:
>>
>>> Hi all,
>>>
>>> can I have reviews for this change that fixes some uses of HeapRegionManager::max_regions()/max_expandable_regions() in
>>> our code to use the correct variant?
>>>
[...]
>> PR: https://git.openjdk.java.net/jdk/pull/178
>
> Looks good.
>
> A couple of questions / comments about pre-existing code. If these are
> indeed things to change, they can/should be new RFEs.
>
> ------------------------------------------------------------------------------
> src/hotspot/share/gc/g1/g1RemSet.cpp
> 488 void G1RemSet::initialize(uint max_reserved_regions) {
> 489 G1FromCardCache::initialize(num_par_rem_sets(), max_reserved_regions);
> 490 _scan_state->initialize(max_reserved_regions);
> 491 }
>
> [pre-existing]
> This seems like an odd place to be initializing all-static G1FromCardCache.
>
> ------------------------------------------------------------------------------
> src/hotspot/share/gc/g1/g1RemSet.cpp
> 29 HeapRegionRange find_unavailable_from_idx(uint index) const;
> ...
> 133 uint find_empty_from_idx_reverse(uint start_idx, uint* res_idx) const;
>
> [pre-existing]
> It seems odd that the second doesn't also return a HeapRegionRange, rather
> than having a return uint and an out parameter.
>
> ------------------------------------------------------------------------------
>
thanks for your review. I created two CRs for these issues.
Thanks,
Thomas
More information about the hotspot-gc-dev
mailing list