RFR: 8252041: G1: Fix incorrect uses of HeapRegionManager::max_length

[Thomas Schatzl]

On Wed, 16 Sep 2020 08:50:38 GMT, Stefan Johansson <sjohanss 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.
>
> Thanks for cleaning this up Thomas.
> 
> The changes look good but I find these two names a bit confusing since that are so similar, what do you think about:
> `max_regions()` -> `reserved_regions()`
> `max_expandable_regions()` -> `max_regions()`
> 
> And similar for the functions in the HeapRegionManager, what do you think about this?

Hi,

On 16.09.20 10:50, Stefan Johansson wrote:
> *@kstefanj* commented on this pull request.
> 
> Thanks for cleaning this up Thomas.
> 
> The changes look good but I find these two names a bit confusing since
> that are so similar, what do you think about:
> |max_regions()| -> |reserved_regions()|
> |max_expandable_regions()| -> |max_regions()|
> 
> And similar for the functions in the HeapRegionManager, what do you
> think about this?
> 

Fine with me - I considered this but then kept this. Thanks for your
early feedback!

Thanks,
   Thomas

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

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