RFR: 8357086: os::xxx functions returning memory size should return size_t [v25]
David Holmes
dholmes at openjdk.org
Wed Aug 6 03:57:10 UTC 2025
On Tue, 5 Aug 2025 08:27:08 GMT, Anton Artemov <duke at openjdk.org> wrote:
>> Hi,
>>
>> in this PR the output value types for functions which return memory are changed, namely:
>>
>>
>> static julong available_memory(); --> static bool available_memory(size_t& value);
>> static julong used_memory(); --> static bool used_memory(size_t& value);
>> static julong free_memory(); --> static bool free_memory(size_t& value);
>> static jlong total_swap_space(); --> static bool total_swap_space(size_t& value);
>> static jlong free_swap_space(); --> static bool free_swap_space(size_t& value);
>> static julong physical_memory(); --> static size_t physical_memory(size_t& value);
>>
>>
>> The return boolean value, where available, indicates success, whereas the actual value is assigned to the input argument. The following recommended usage pattern is introduced: where applicable, an unsuccessful call is logged.
>>
>> `ATTRIBUTE_NODISCARD` macro is added as a placeholder for `[[nodiscard]]`, which will be available with C++17.
>>
>> Tested in GHA and Tiers 1-5.
>
> Anton Artemov has updated the pull request incrementally with one additional commit since the last revision:
>
> 8357086: Addressed reviewer's comments
src/hotspot/os/linux/os_linux.cpp line 299:
> 297: jlong memory_and_swap_limit_in_bytes = OSContainer::memory_and_swap_limit_in_bytes();
> 298: jlong memory_limit_in_bytes = OSContainer::memory_limit_in_bytes();
> 299: value = static_cast<size_t>(memory_and_swap_limit_in_bytes - memory_limit_in_bytes);
This isn't what I meant. You want to save into the locals before each of the `if` statements, else you are still calling these functions twice.
src/hotspot/os/windows/os_windows.cpp line 869:
> 867: return true;
> 868: } else {
> 869: errno = ::GetLastError();
Why are you setting `errno` here and elsewhere? I see a couple of places where we do do that though I do not know why, but generally we just want to save the `::GetlastError()` value (here we do need a local) and print it.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/25450#discussion_r2255787665
PR Review Comment: https://git.openjdk.org/jdk/pull/25450#discussion_r2255792577
More information about the hotspot-dev
mailing list