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

[Joel Sikström]

On Wed, 20 Aug 2025 07:19:36 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/windows/os_windows.cpp line 4167:

> 4165:   BOOL res = GlobalMemoryStatusEx(&ms);
> 4166:   if (res != TRUE)
> 4167:   {

To fit in with the surrounding style, the '{' should be moved to the same line as the if-statement.
Suggestion:

  if (res != TRUE) {

src/hotspot/share/gc/shared/gcInitLogger.cpp line 66:

> 64: void GCInitLogger::print_memory() {
> 65:   size_t memory = os::physical_memory();
> 66:   log_info_p(gc, init)("Memory: %zu%s", byte_size_in_proper_unit(memory), proper_unit_for_byte_size(memory));

Here as well.

src/hotspot/share/gc/z/zLargePages.cpp line 35:

> 33: 
> 34:   const size_t memory = os::physical_memory();
> 35:   log_info_p(gc, init)("Memory: %zu%s", byte_size_in_proper_unit(memory), proper_unit_for_byte_size(memory));

I would like to see PROPERFMT + PROPERFMTARGS here instead of the expanded version.

src/hotspot/share/runtime/os.hpp line 343:

> 341: 
> 342:   static size_t physical_memory();
> 343:   static bool has_allocatable_memory_limit(size_t* limit);

This is likely a mistake from merging.

`static bool has_allocatable_memory_limit(size_t* limit)` was renamed to `static size_t commit_memory_limit()` in [JDK-8364248](https://bugs.openjdk.org/browse/JDK-8364248), so the definition should be removed here.

src/hotspot/share/utilities/globalDefinitions.hpp line 1367:

> 1365: 
> 1366: //----------------------------------------------------------------------------------------------------
> 1367: 

Now I'm bit picky, but I think one line extra here is enough.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/25450#discussion_r2288552444
PR Review Comment: https://git.openjdk.org/jdk/pull/25450#discussion_r2288548579
PR Review Comment: https://git.openjdk.org/jdk/pull/25450#discussion_r2288489717
PR Review Comment: https://git.openjdk.org/jdk/pull/25450#discussion_r2288563780
PR Review Comment: https://git.openjdk.org/jdk/pull/25450#discussion_r2288562309