RFR: JDK-8310107: os::trace_page_sizes_for_requested_size should name alignment as requested page size [v2]

[Stefan Karlsson]

On Fri, 16 Jun 2023 14:18:07 GMT, Thomas Stuefe <stuefe at openjdk.org> wrote:

>> Somewhat trivial renaming patch.
>> 
>> `os::trace_page_sizes_for_requested_size()` names "alignment" what is actually the requested page size. Historically, the distinction between "alignment" and "page size" in memory reservation code has not been clear. We often use "alignment" where we mean "page size". OTOH, ReservedSpace constructor takes a "preferred page size" argument, which it then uses as both alignment request and pagesize request, meaning that even if it fails to get the desired page size it attempts to satisfy the alignment.
>> 
>> Clearing this up would be a larger effort. Here, I propose to rename "alignment" to "requested page size" in `os::trace_page_sizes_for_requested_size()` since it clearer conveys the difference between requested and actual page size, which is the important bit for pagesize logging.
>> 
>> ---
>> 
>> Patch renames "alignment" to "requested pagesize" in printout and code.
>> 
>> It also reshuffles the parameters somewhat to have requested properties at the beginning, followed by the actual properties of the region.
>> 
>> Printout before, for a failed 2M paged allocation. Notice the difference between requested and actual page size, the latter being labeled as "alignment"):
>> 
>> 
>> OpenJDK 64-Bit Server VM warning: Failed to reserve and commit memory using large pages. req_addr: 0x0000000000000000 bytes: 2097152
>> [0.012s][info][pagesize] Block Offset Table: req_size=2M base=0x00007f3518200000 page_size=4K alignment=2M size=2M
>> 
>> 
>> Printout now:
>> 
>> 
>> OpenJDK 64-Bit Server VM warning: Failed to reserve and commit memory using large pages. req_addr: 0x0000000000000000 bytes: 2097152
>> [0.011s][info][pagesize] Block Offset Table: req_size=2M req_page_size=2M base=0x00007fda0e400000 size=2M page_size=4K
>
> Thomas Stuefe has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Feedback Stefan

Looks good. One nit:

src/hotspot/share/runtime/os.hpp line 408:

> 406:   // call.  The (optional) base and size parameters should come from the
> 407:   // ReservedSpace base() and size() methods.
> 408: //  static void trace_page_sizes(const char* str, const size_t* page_sizes, int count);

Should this be removed?

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

Marked as reviewed by stefank (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/14484#pullrequestreview-1484105605
PR Review Comment: https://git.openjdk.org/jdk/pull/14484#discussion_r1232598052