RFR: 8367413: Refactor types in Arguments::set_heap_size() [v5]
Leo Korinth
lkorinth at openjdk.org
Wed Oct 8 09:03:41 UTC 2025
On Tue, 7 Oct 2025 09:34:19 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:
>
> Rename ram_limit_set to has_ram_limit
src/hotspot/share/utilities/globalDefinitions.hpp line 1112:
> 1110: return (size_t)MIN2(value, (T)std::numeric_limits<size_t>::max());
> 1111: }
> 1112:
Maybe something more generic like this:
// return the value of B clamped by the numeric limits of S, casted to S
template<class S, class B>
S clamp_into_from(B value) {
static_assert(std::numeric_limits<B>::min() <= std::numeric_limits<S>::min(), "make sure the big type min can hold the small type min");
static_assert(std::numeric_limits<B>::max() >= std::numeric_limits<S>::max(), "make sure the big type max can hold the small type max");
B v = clamp<B>(value, std::numeric_limits<S>::min(), std::numeric_limits<S>::max());
return static_cast<S>(v);
}
and place it after clamp?
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/27224#discussion_r2413130935
More information about the hotspot-gc-dev
mailing list