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

[Anton Artemov]

On Wed, 30 Jul 2025 14:42:33 GMT, Stefan Karlsson <stefank at openjdk.org> wrote:

>> Anton Artemov has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   8357086: Addressed reviewer's comments
>
> src/hotspot/share/gc/shared/gcInitLogger.cpp line 71:
> 
>> 69:     log_info_p(gc, init)("Memory: NA");
>> 70:   }
>> 71:   julong memory = static_cast<julong>(phys_mem);
> 
> Something went wrong here. The old logging is still left. Could you get rid of size_t ?`phys_mem` and only have a `size_t memory` variable?

Yes, missed that, removed.

> src/hotspot/share/gc/z/zLargePages.cpp line 37:
> 
>> 35:     log_info_p(gc, init)("Memory: %zu%s", byte_size_in_proper_unit(phys_mem), proper_unit_for_byte_size(phys_mem));
>> 36:   }
>> 37:   else {
> 
> Suggestion:
> 
>   } else {

Fixed.

> src/hotspot/share/gc/z/zLargePages.cpp line 40:
> 
>> 38:     log_info_p(gc, init)("Memory: NA");
>> 39:   }
>> 40:   log_info_p(gc, init)("Memory: %zuM", phys_mem / M);
> 
> There's double-logging here now.

Yes, missed that, removed.

> src/hotspot/share/runtime/arguments.cpp line 1525:
> 
>> 1523:       if (!os::physical_memory(physical_mem_val)) {
>> 1524:         log_debug(os)("os::physical_memory() failed");
>> 1525:       }
> 
> Suggestion:
> 
>       // Ignore return value
>       (void)os::physical_memory(physical_mem_val);

Removed.

> src/hotspot/share/runtime/arguments.cpp line 1535:
> 
>> 1533:     if (!os::physical_memory(physical_mem_val)) {
>> 1534:       log_debug(os)("os::physical_memory() failed");
>> 1535:     }
> 
> Suggestion:
> 
>     // Ignore return value
>     (void)os::physical_memory(physical_mem_val);

Removed.

> src/hotspot/share/runtime/os.cpp line 1189:
> 
>> 1187:   if (!physical_memory(phys_mem)) {
>> 1188:     log_debug(os)("os::physical_memory() failed");
>> 1189:   }
> 
> Suggestion:
> 
>   // Ignore return value
>   (void)physical_memory(phys_mem);

Removed.

> src/hotspot/share/runtime/os.cpp line 1950:
> 
>> 1948:   if (!os::physical_memory(phys_mem)) {
>> 1949:     log_debug(os)("os::physical_memory() failed");
>> 1950:   }
> 
> Suggestion:
> 
>   // Ignore return value
>   (void)os::physical_memory(phys_mem);

Removed.

> src/hotspot/share/services/heapDumper.cpp line 2618:
> 
>> 2616:     if (!os::free_memory(free_memory)) {
>> 2617:       log_debug(heapdump)("os::free_memory() failed");
>> 2618:     }
> 
> Suggestion:
> 
>     (void)os::free_memory(free_memory);

Removed.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/25450#discussion_r2243015718
PR Review Comment: https://git.openjdk.org/jdk/pull/25450#discussion_r2243015527
PR Review Comment: https://git.openjdk.org/jdk/pull/25450#discussion_r2243015153
PR Review Comment: https://git.openjdk.org/jdk/pull/25450#discussion_r2243013502
PR Review Comment: https://git.openjdk.org/jdk/pull/25450#discussion_r2243013337
PR Review Comment: https://git.openjdk.org/jdk/pull/25450#discussion_r2243013171
PR Review Comment: https://git.openjdk.org/jdk/pull/25450#discussion_r2243012954
PR Review Comment: https://git.openjdk.org/jdk/pull/25450#discussion_r2243012593