RFR: 8357086: os::xxx functions returning memory size should return size_t [v28]

[David Holmes]

On Tue, 19 Aug 2025 08:25:13 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

Asserts should always provide as much error detail as is available - otherwise when they fail we do not know why.

Thanks

src/hotspot/os/linux/os_linux.cpp line 306:

> 304:   int ret = sysinfo(&si);
> 305:   if (ret != 0) {
> 306:     assert(false, "sysinfo failed in total_swap_space()?");

We should report the value of errno in these cases.
Suggestion:

    assert(false, "sysinfo failed in total_swap_space(): %s", os::strerror(errno));

src/hotspot/os/windows/os_windows.cpp line 869:

> 867:     return true;
> 868:   } else {
> 869:     assert(false, "GlobalMemoryStatusEx failed in win32::available_memory().");

These should report the value of `::GetLastError()`

src/hotspot/os/windows/os_windows.cpp line 870:

> 868:   } else {
> 869:     assert(false, "GlobalMemoryStatusEx failed in win32::available_memory().");
> 870:     return res == TRUE;

Suggestion:

    return false;

Please restore all these to "return false" as we know that `res == false` here.

-------------

Changes requested by dholmes (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/25450#pullrequestreview-3134449354
PR Review Comment: https://git.openjdk.org/jdk/pull/25450#discussion_r2286731388
PR Review Comment: https://git.openjdk.org/jdk/pull/25450#discussion_r2286734810
PR Review Comment: https://git.openjdk.org/jdk/pull/25450#discussion_r2286736956