RFR: 8243315: ParallelScavengeHeap::initialize() passes GenAlignment as page size to os::trace_page_sizes instead of actual page size

[Thomas Stuefe]

On Wed, 25 Nov 2020 13:23:30 GMT, Stefan Johansson <sjohanss at openjdk.org> wrote:

>> I wrote
>> 
>>>  On Linux, we have e.g. int getpagesize(void);
>>> 
>> 
>> which is of course wrong since it just returns the global small page size. Querying the page size of an arbitrary range seems to require walking smaps, and that is way more cumbersome.
>
> Hi Thomas, 
> 
> Thanks for sharing your concerns. I agree that this becomes more of a problem now when it is exposed outside the GC, that's why I wasn't sure where we should put this:
>> Not entirely sure where the best location for such helper is, but a static function ReservedSpace::actual_page_size(ReservedSpace) could work.
> 
> This is also why it was added as a static helper rather than a member. An alternative is to move this helper to a shared GC utility, and maybe document it a bit more. Adding multiple large page sizes will require this function to be updated, that's one of the reasons I wanted it shared not duplicated. 
> 
> As you say the best thing would be if we had a way to really query the page size rather than doing a good estimation given the looks of the reserved space. But for the current use-cases in the GC, this estimation seems to be enough. Going the other route is, as you say, way more work.
> 
> A different solution would be to add a `_largest_page_size` member to the reserved space, which would be the larges page size used by that reservation. That might make sense if we are moving towards having multiple large page sizes.
> 
> What would be your preferred way forward?
> 
> Cheers,
> Stefan

Hi Stefan,

> Hi Thomas,
> 
> Thanks for sharing your concerns. I agree that this becomes more of a problem now when it is exposed outside the GC, that's why I wasn't sure where we should put this:
> 
> > Not entirely sure where the best location for such helper is, but a static function ReservedSpace::actual_page_size(ReservedSpace) could work.
> 
> This is also why it was added as a static helper rather than a member. An alternative is to move this helper to a shared GC utility, and maybe document it a bit more. Adding multiple large page sizes will require this function to be updated, that's one of the reasons I wanted it shared not duplicated.
> 
> As you say the best thing would be if we had a way to really query the page size rather than doing a good estimation given the looks of the reserved space. But for the current use-cases in the GC, this estimation seems to be enough. Going the other route is, as you say, way more work.
> 
> A different solution would be to add a `_largest_page_size` member to the reserved space, which would be the larges page size used by that reservation. That might make sense if we are moving towards having multiple large page sizes.
> 
> What would be your preferred way forward?
> 
> Cheers,
> Stefan

Thanks for taking my concerns seriously. I know that this is all a bit messy, and has a long history.

I briefly looked into some way to ask the OS about the page size of a memory region. That is certainly possible but annoyingly complicated.

On AIX we have a thing already. The only platform that has :)

On Linux you would have to scan /proc/self/smaps. May be worthwhile but complex and expensive for large processes.

On Windows I have no clue what do do. VirtualQuery() does not return page information. Neither do I know what to do on MacOS.

Interestingly, I found that we have os::page_info() and os::scan_pages(), which look like they would do what we want but they are empty on all platforms. Solaris leftover. I filed JDK-8257076 for you guys to track this since you may remove some of the associated NUMA coding too.

---

So the more practical and faster way would be to store this information ourselves. It would be better than trying to deduce what we did at reservation time.

I am currently working on a prototype to expand os::reserve_memory_special() to optionally return information about the page sizes in the reserved range. Kinda like sigset_t. That would allow the os implementors to do whatever (eg mix in 2M pages), and you have the information about that set of page sizes, which could be stored in ReservedSpace as a member. Later, that info can be used as "what the underlying page sizes are to our best knowledge".

That prototype would look a bit like this:

char* os::reserve_memory_special(..., pagesizeset_t* page_sizes = NULL);

This is a smaller version of what I think would be generally a good idea, which is for all os::reserve__... APIs to return meta information about the reserved space for the caller to hold on to. But this is only for os::reserve_memory_special(), and only for page sizes, so less daunting.

I'll post an RFR if I have a prototype ready. Then we may take another look at this change and rework this actual_page_size().

Cheers, Thomas

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

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