RFR: 8252041: G1: Fix incorrect uses of HeapRegionManager::max_length [v4]

[Kim Barrett]

> 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?
>> 
>> HeapRegionManager::max_length gives the absolute maximum number of regions reserved for the heap based on the
>> reservation.
>> HeapRegionmanager::max_expandable_regions() returns the maximum number of regions the heap can grow to (can be
>> committed).
>> Typically they are exchangeable, but if the reservation is larger than what -Xmx allows (like when file-mapping old
>> gen), this is not the case, and causes assertions. In some cases the existing region manager code could (if it reserved
>> more than -Xmx) give back more than -Xmx allows.  Testing: tier1-5.
> 
> Thomas Schatzl has updated the pull request incrementally with three additional commits since the last revision:
> 
> - sjohanss review: Fix comments
> - Merge branch '8252041-incorrect-use-of-max-length' of gh:tschatzl/jdk into 8252041-incorrect-use-of-max-length
> - Merge branch '8252041-incorrect-use-of-max-length' of gh:tschatzl/jdk
> 
> -------------
> 
> Changes:
>  - all: https://git.openjdk.java.net/jdk/pull/178/files
>  - new: https://git.openjdk.java.net/jdk/pull/178/files/54673e95..f0b252b5
> 
> Webrevs:
> - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=178&range=03
> - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=178&range=02-03
> 
>  Stats: 2 lines in 1 file changed: 0 ins; 0 del; 2 mod
>  Patch: https://git.openjdk.java.net/jdk/pull/178.diff
>  Fetch: git fetch https://git.openjdk.java.net/jdk pull/178/head:pull/178
> 
> 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.

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