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