From 4e2cc725f4d3a11a01853040555885b4891651db Mon Sep 17 00:00:00 2001 From: Robert Toyonaga Date: Thu, 9 Apr 2026 16:56:12 -0400 Subject: [PATCH] split up map_or_reserve_memory_aligned. Small fixes. --- src/hotspot/os/windows/os_windows.cpp | 58 ++++++++++++++----- .../hotspot/gtest/runtime/test_os_windows.cpp | 2 - 2 files changed, 42 insertions(+), 18 deletions(-) diff --git a/src/hotspot/os/windows/os_windows.cpp b/src/hotspot/os/windows/os_windows.cpp index 06c996c0ddb..db5681ecbde 100644 --- a/src/hotspot/os/windows/os_windows.cpp +++ b/src/hotspot/os/windows/os_windows.cpp @@ -3291,7 +3291,7 @@ static bool is_MapViewOfFile3_supported() { // Multiple threads can race in this code but it's not possible to unmap small sections of // virtual space to get requested alignment, like posix-like os's. // Windows prevents multiple thread from remapping over each other so this loop is thread-safe. -static char* map_or_reserve_memory_aligned(size_t size, size_t alignment, int file_desc, MemTag mem_tag) { +static char* reserve_memory_aligned(size_t size, size_t alignment, MemTag mem_tag) { assert(is_aligned(alignment, os::vm_allocation_granularity()), "Alignment must be a multiple of allocation granularity"); assert(is_aligned(size, os::vm_allocation_granularity()), @@ -3304,24 +3304,50 @@ static char* map_or_reserve_memory_aligned(size_t size, size_t alignment, int fi static const int max_attempts = 20; for (int attempt = 0; attempt < max_attempts && aligned_base == nullptr; attempt ++) { - char* extra_base = file_desc != -1 ? os::map_memory_to_file(extra_size, file_desc, mem_tag) : - os::reserve_memory(extra_size, mem_tag); + char* extra_base = os::reserve_memory(extra_size, mem_tag); if (extra_base == nullptr) { return nullptr; } // Do manual alignment aligned_base = align_up(extra_base, alignment); - - if (file_desc != -1) { - os::unmap_memory(extra_base, extra_size); - } else { - os::release_memory(extra_base, extra_size); - } + os::release_memory(extra_base, extra_size); // Attempt to map, into the just vacated space, the slightly smaller aligned area. // Which may fail, hence the loop. - aligned_base = file_desc != -1 ? os::attempt_map_memory_to_file_at(aligned_base, size, file_desc, mem_tag) : - os::attempt_reserve_memory_at(aligned_base, size, mem_tag); + aligned_base = os::attempt_reserve_memory_at(aligned_base, size, mem_tag); + } + + assert(aligned_base != nullptr, + "Did not manage to re-map after %d attempts (size %zu, alignment %zu)", max_attempts, size, alignment); + + return aligned_base; +} + +// Similar to reserve_memory_aligned, multiple threads can race in this code. +static char* map_memory_aligned(size_t size, size_t alignment, int file_desc, MemTag mem_tag) { + assert(is_aligned(alignment, os::vm_allocation_granularity()), + "Alignment must be a multiple of allocation granularity"); + assert(is_aligned(size, os::vm_allocation_granularity()), + "Size must be a multiple of allocation granularity"); + + size_t extra_size = size + alignment; + assert(extra_size >= size, "overflow, size is too large to allow alignment"); + + char* aligned_base = nullptr; + static const int max_attempts = 20; + + for (int attempt = 0; attempt < max_attempts && aligned_base == nullptr; attempt ++) { + char* extra_base = os::map_memory_to_file(extra_size, file_desc, mem_tag); + if (extra_base == nullptr) { + return nullptr; + } + // Do manual alignment + aligned_base = align_up(extra_base, alignment); + os::unmap_memory(extra_base, extra_size); + + // Attempt to map, into the just vacated space, the slightly smaller aligned area. + // Which may fail, hence the loop. + aligned_base = os::attempt_map_memory_to_file_at(aligned_base, size, file_desc, mem_tag); } assert(aligned_base != nullptr, @@ -3368,8 +3394,8 @@ static char* map_memory_aligned_va2(size_t size, size_t alignment, int file_desc MemTracker::record_virtual_memory_reserve_and_commit(aligned_base, size, CALLER_PC, mem_tag); return aligned_base; } - log_trace(os)("Aligned allocation via VirtualAlloc2/MapViewOfFile3 failed, falling back to retry loop. GetLastError->%lu.", GetLastError()); - return map_or_reserve_memory_aligned(size, alignment, file_desc, mem_tag); + log_trace(os)("Aligned allocation via MapViewOfFile3 failed, falling back to retry loop. GetLastError->%lu.", GetLastError()); + return map_memory_aligned(size, alignment, file_desc, mem_tag); } // VirtualAlloc2 supports alignment natively. @@ -3403,7 +3429,7 @@ static char* reserve_memory_aligned_va2(size_t size, size_t alignment, MemTag me return aligned_base; } log_trace(os)("Aligned allocation via VirtualAlloc2 failed, falling back to retry loop. GetLastError->%lu.", GetLastError()); - return map_or_reserve_memory_aligned(size, alignment, -1/* file_desc */, mem_tag); + return reserve_memory_aligned(size, alignment, mem_tag); } size_t os::commit_memory_limit() { @@ -3456,14 +3482,14 @@ char* os::reserve_memory_aligned(size_t size, size_t alignment, MemTag mem_tag, if (is_VirtualAlloc2_supported()) { return reserve_memory_aligned_va2(size, alignment, mem_tag); } - return map_or_reserve_memory_aligned(size, alignment, -1/* file_desc */, mem_tag); + return reserve_memory_aligned(size, alignment, mem_tag); } char* os::map_memory_to_file_aligned(size_t size, size_t alignment, int fd, MemTag mem_tag) { if (is_MapViewOfFile3_supported()) { return map_memory_aligned_va2(size, alignment, fd, mem_tag); } - return map_or_reserve_memory_aligned(size, alignment, fd, mem_tag); + return map_memory_aligned(size, alignment, fd, mem_tag); } char* os::pd_reserve_memory(size_t bytes, bool exec) { diff --git a/test/hotspot/gtest/runtime/test_os_windows.cpp b/test/hotspot/gtest/runtime/test_os_windows.cpp index 14a7a527b8e..4ac2759c17e 100644 --- a/test/hotspot/gtest/runtime/test_os_windows.cpp +++ b/test/hotspot/gtest/runtime/test_os_windows.cpp @@ -869,8 +869,6 @@ TEST_VM(os_windows, numa_placeholder_reserve_commit) { char* result = os::attempt_reserve_memory_at(nullptr, size, mtTest); ASSERT_TRUE(result != nullptr) << "Failed to reserve memory"; - - ASSERT_TRUE(is_aligned(result, os::vm_allocation_granularity())); ASSERT_TRUE(os::commit_memory(result, size, false));