RFR: 8367413: Refactor types in Arguments::set_heap_size() [v3]
Joel Sikström
jsikstro at openjdk.org
Thu Oct 2 13:33:00 UTC 2025
On Thu, 2 Oct 2025 13:30:02 GMT, Joel Sikström <jsikstro at openjdk.org> wrote:
>> Hello,
>>
>> There are several integer types used when setting the heap size ergonomically in `Arguments::set_heap_size()`, such as julong, uint64_t and size_t. It's not clear if this code works as intended on a 32-bit VM with more than 4GB physical memory. There might be issues when converting to/from size_t and uint64_t that we don't handle properly. I suggest we should be more robust and have more control over transitions between potentially smaller types.
>>
>> Additionally, I've gone ahead and added comments which I think makes it easier to understand what's going on in this function. I've tried my best to leave the existing behavior unchanged, apart from type conversion.
>>
>> When looking at this I tried to get my head around the default value of the MaxRAM flag. Since this flag is closely coupled with setting the heap size, I've also gone ahead and removed some of the 32-bit (x86, Windows) code in relation to this flag.
>>
>>
>> Testing:
>> * Oracle's tier1-8 on all Oracle supported platforms
>
> Joel Sikström has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains seven commits:
>
> - Namings and comment
> - 8367413: Refactor types in Arguments::set_heap_size()
> - Merge branch 'master' into JDK-8367413_arguments_sizet_julong
> - Revert "8367413: Use size_t instead of julong in runtime/arguments.cpp"
>
> This reverts commit 35c6057a4b5f0e478f3e703f6fa14b137cd73ca8.
> - Revert "size_t casts for 32-bit part of test_arguments.cpp"
>
> This reverts commit dba2ab4699b9295ba406abf174a7829600ce8625.
> - size_t casts for 32-bit part of test_arguments.cpp
> - 8367413: Use size_t instead of julong in runtime/arguments.cpp
[JDK-8367485](https://bugs.openjdk.org/browse/JDK-8367485) is now integrated. I've changed the direction of this issue a bit. I now focus on makus sure the transition from uint64_t to size_t is handled robustly along with adding comments to make things easier to understand in `Arguments::set_heap_size()`.
Setting `MaxRAM` to `MIN2(MaxRAM, (uint64_t) ms.ullTotalVirtual)` on Windows was only needed on 32-bit, which is no longer supported. So I've gone ahead and removed it. The virtual address space will practically always be extremely large on Windows 64-bit. See the following resources for more information:
https://learn.microsoft.com/en-us/windows/win32/memory/memory-limits-for-windows-releases
https://learn.microsoft.com/en-us/windows-server/get-started/locks-limits?tabs=full-comparison&pivots=windows-server-2025
-------------
PR Comment: https://git.openjdk.org/jdk/pull/27224#issuecomment-3361236605
More information about the hotspot-gc-dev
mailing list