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

Stefan Johansson sjohanss at openjdk.java.net
Thu Sep 17 07:00:34 UTC 2020


On Wed, 16 Sep 2020 14:59:44 GMT, Thomas Schatzl <tschatzl at openjdk.org> 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 with a new target base due to a merge or a rebase. The pull request now
> contains three commits:
>  - Merge branch 'master' into 8252041-incorrect-use-of-max-length
>  - Renaming
>  - Initial import

Looks good apart from two comments which still uses the max-prefix for `reserved_length()`.

I'll mark this as approved, just fix those comments before pushing.

src/hotspot/share/gc/g1/heapRegionManager.hpp line 128:

> 126:   // Finds the next sequence of unavailable regions starting at the given index. Returns the
> 127:   // sequence found as a HeapRegionRange. If no regions can be found, both start and end of
> 128:   // the returned range is equal to max_reserved_length().

Same as above, drop "max_".

src/hotspot/share/gc/g1/heapRegionManager.hpp line 83:

> 81: //   number of regions+1 for which we have HeapRegions.
> 82: // * max_length() returns the maximum number of regions the heap may commit.
> 83: // * max_reserved_length() returns the maximum number of regions the heap has reserved.

This should say "`reserved_length()` returns..."

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

Marked as reviewed by sjohanss (Reviewer).

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



More information about the hotspot-gc-dev mailing list