RFR: 8367413: Refactor types in Arguments::set_heap_size() [v4]
Albert Mingkun Yang
ayang at openjdk.org
Tue Oct 7 09:23:26 UTC 2025
On Tue, 7 Oct 2025 09:15:34 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.
>>
>> Testing:
>> * Oracle's tier1-8 on all Oracle supported platforms
>
> Joel Sikström has updated the pull request incrementally with one additional commit since the last revision:
>
> Revert MaxRAM default removals, defer to future enhancement
src/hotspot/share/runtime/arguments.cpp line 1514:
> 1512:
> 1513: // Check if the user has configured any limit on the amount of RAM we may use.
> 1514: bool ram_limit_set = !FLAG_IS_DEFAULT(MaxRAMPercentage) ||
How about `has_ram_limit`/`is_ram_limit_set`?
src/hotspot/share/runtime/arguments.cpp line 1544:
> 1542: uint64_t max_memory = (uint64_t)(((double)physical_memory * MaxRAMPercentage) / 100);
> 1543:
> 1544: const size_t reasonable_min = limit_by_size_t_max(min_memory);
I wonder whether we should emit a log-info/warning, if `limit_by_size_t_max` does change the result.
src/hotspot/share/runtime/arguments.cpp line 1590:
> 1588:
> 1589: if (UseCompressedOops) {
> 1590: size_t heap_end = HeapBaseMinAddress + MaxHeapSize;
I wonder if this is a pre-existing issue: `HeapBaseMinAddress` , according to its name, sounds like a `uintptr_t` instead of `size_t`.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/27224#discussion_r2409967306
PR Review Comment: https://git.openjdk.org/jdk/pull/27224#discussion_r2409970585
PR Review Comment: https://git.openjdk.org/jdk/pull/27224#discussion_r2409973796
More information about the hotspot-gc-dev
mailing list