RFR: 8266349: Pass down requested page size to reserve_memory_special [v2]

[Stefan Johansson]

On Wed, 5 May 2021 13:50:55 GMT, Thomas Stuefe <stuefe at openjdk.org> wrote:

>>> Very good.
>>> 
>> 
>> Thanks =)
>> 
>>> This is not only cosmetic but functional, no? Since before we would always os::large_page_size(); now, in the case of ReservedSpace::ReservedSpace(size), we use whatever os::page_size_for_region wants us to use. Lets see if any errors creep up.
>>> 
>> 
>> I think this should not change functionality. Before I started my refactoring we did not have `ReservedSpace::ReservedSpace(size)` but this function below where the second parameter was optional. As you can see, if no `preferred_page_size` was given we would use `os::page_size_for_region_unaligned(size, 1)` which is what now is done in `ReservedSpace::ReservedSpace(size)`:
>> 
>> ReservedSpace::ReservedSpace(size_t size, size_t preferred_page_size) : _fd_for_heap(-1) {
>>   bool has_preferred_page_size = preferred_page_size != 0;
>>   // Want to use large pages where possible and pad with small pages.
>>   size_t page_size = has_preferred_page_size ? preferred_page_size : os::page_size_for_region_unaligned(size, 1);
>>   bool large_pages = page_size != (size_t)os::vm_page_size();
>>   ...
>> 
>> 
>> And if we end up with a page size that isn't large we will never get down into `reserve_memory_special(...)`. Does this sound correct, or did I miss your point?
>
>> > Very good.
>> 
>> Thanks =)
>> 
>> > This is not only cosmetic but functional, no? Since before we would always os::large_page_size(); now, in the case of ReservedSpace::ReservedSpace(size), we use whatever os::page_size_for_region wants us to use. Lets see if any errors creep up.
>> 
>> I think this should not change functionality. Before I started my refactoring we did not have `ReservedSpace::ReservedSpace(size)` but this function below where the second parameter was optional. As you can see, if no `preferred_page_size` was given we would use `os::page_size_for_region_unaligned(size, 1)` which is what now is done in `ReservedSpace::ReservedSpace(size)`:
>> 
>> ```
>> ReservedSpace::ReservedSpace(size_t size, size_t preferred_page_size) : _fd_for_heap(-1) {
>>   bool has_preferred_page_size = preferred_page_size != 0;
>>   // Want to use large pages where possible and pad with small pages.
>>   size_t page_size = has_preferred_page_size ? preferred_page_size : os::page_size_for_region_unaligned(size, 1);
>>   bool large_pages = page_size != (size_t)os::vm_page_size();
>>   ...
>> ```
>> 
>> And if we end up with a page size that isn't large we will never get down into `reserve_memory_special(...)`. Does this sound correct, or did I miss your point?
> 
> No, this sounds right.

Thanks @tstuefe and @mgkwill for reviewing.

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

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