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