RFR(S): 8209162: Page size selection does not always select optimal page size

[Ivan Walulya]

Hi Stefan,

Yes, that would make the code clearer given that  currently page_sz = MIN2(max_page_sz, min_page_sz);. And I cannot find the motivation for the "min 8 pages” used for max_page_sz. 

//Ivan

> On 21 Apr 2020, at 13:27, Stefan Johansson <stefan.johansson at ORACLE.COM> wrote:
> 
> On 2020-04-21 13:18, Stefan Johansson wrote:
>> Hi Ivan,
>> I'm not very familiar with this code, but one observation. This is a bit similar to what I looked at recently for G1. There the region size was different for a given heap size depending on if Xms was set or not. Here we can run into a situation where the page size differ depending on Xms, and this also feels a bit strange. Would there be a problem to simplify this to a single line and always just care about min:
>> const size_t page_sz = os::page_size_for_region_aligned(MinHeapSize, 4);
>> I'm trying to understand when this would be a problem, but can't think of anything straight away.
> It would still be true that for a given max heap we might use different page sizes depending in Xms, but at least now it would be clear from the calculation that the max is not considered at all.
> 
>> Cheers,
>> Stefan
>> On 2020-04-18 12:04, Ivan Walulya wrote:
>>> 
>>> 
>>>> On 17 Apr 2020, at 12:40, Bernd Eckenfels <ecki at zusammenkunft.net> wrote:
>>>> 
>>>> It looks rather confusing to use the min_pages variable for two sizes, maybe better call it pages?
>>> 
>>> Thanks for pointing this out. Refactored, hope it is clearer now.
>>> https://cr.openjdk.java.net/~iwalulya/8209162/01/ <https://cr.openjdk.java.net/~iwalulya/8209162/01/>
>>> 
>>>> 
>>>> BTW i am not sure why the minimum number of pages does not apply to the static side heap case. Isn't there requirement fūr the 4 generations present in all cases? Maybe the comment can describe this more cleanly.
>>> 
>>> const size_t page_sz = MIN2(max_page_sz, min_page_sz); would handle this case.
>>>   I hope the refactored code is less confusing.
>>> 
>>>> 
>>>> 
>>>> -- 
>>>> http://bernd.eckenfels.net
>>>