RFR: 8253683: Clean up and clarify uses of os::vm_allocation_granularity

[Frederic Thevenet]

On Tue, 25 Nov 2025 14:31:39 GMT, Casper Norrbin <cnorrbin at openjdk.org> wrote:

> Hi everyone,
> 
> `os::vm_allocation_granularity()` is meant to describe the alignment restrictions of the operating system when we reserve memory. That is 64 KiB on Windows (`VirtualAlloc`) and 256 MiB on AIX (with `shmat`). On every other platform it happens to match the page size. The page size (available via `os::vm_page_size()`) is what matters when we later commit or protect the reserved pages.
> 
> Because the functions are poorly documented and the two numbers are identical on most systems, they have gradually been used more and more interchangeably. We now have many code paths that round **sizes** up to `os::vm_allocation_granularity()` or assert that a size is a multiple of it. That is wrong. Only addresses need that alignment, sizes merely have to be page-aligned. Places that round sizes should instead use `os::vm_page_size()` as they are unrelated to attach alignment.
> 
> For this change I have gone over the call sites of `os::vm_allocation_granularity()` and where it was being used to pad or sanity-check a size I have instead replaced it with `os::vm_page_size()`. The calls that genuinely deal with an attach address are left untouched.
> 
> Testing:
> - Oracle tiers 1-8

Please note that  I am not a formal reviewer for the JDK project, but I thought I'd chip in and do some of the leg work. I hope this is useful.

src/hotspot/os/posix/os_posix.cpp line 458:

> 456: 
> 457: static size_t calculate_aligned_extra_size(size_t size, size_t alignment) {
> 458:   assert(is_aligned(alignment, os::vm_allocation_granularity()),

Shouldn't `alignment` be checked as  a multiple of page size rather than alloc granularity as well?
I don't see it being used for aligning an address.

src/hotspot/os/windows/os_windows.cpp line 2997:

> 2995:   char * p_buf;
> 2996:   // note: at setup time we guaranteed that NUMAInterleaveGranularity was aligned up to a page size
> 2997:   size_t page_size = UseLargePages ? _large_page_size : os::vm_page_size();

I don't think using `vm_page_size` instead of `vm_allocation_granularity` is appropriate here, as `page_size` is later used to align an address to pass to `virtualAlloc` (`next_alloc_addr`), not just region sizes (see lines 3023-3030)

src/hotspot/share/jfr/recorder/storage/jfrVirtualMemory.cpp line 117:

> 115:   assert(_rs.size() != 0, "invariant");
> 116:   assert(is_aligned(_rs.base(), os::vm_allocation_granularity()), "invariant");
> 117:   assert(is_aligned(_rs.size(), os::vm_page_size()), "invariant");

The assert at line 117 is now redundant with the one done at 125.
AFAICT, neither `os::trace_page_sizes` nor `MemTracker::record_virtual_memory_tag`  have side effects that could affect `_rs.size()`, so line 125 could probably be removed.

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

PR Review: https://git.openjdk.org/jdk/pull/28493#pullrequestreview-3534618037
PR Review Comment: https://git.openjdk.org/jdk/pull/28493#discussion_r2585008814
PR Review Comment: https://git.openjdk.org/jdk/pull/28493#discussion_r2584750844
PR Review Comment: https://git.openjdk.org/jdk/pull/28493#discussion_r2588354727