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

[Anton Artemov]

On Wed, 20 Aug 2025 15:33:02 GMT, Joel Sikström <jsikstro 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/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) {

Thanks for spotting, addressed.

> 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.

Addressed.

> 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.

Addressed.

> 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.

Removed.

> 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.

I removed all added lines, it is a leftover from the initial placing of the `ATTRIBUTE_NODISCARD` dummy placeholder.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/25450#discussion_r2290344867
PR Review Comment: https://git.openjdk.org/jdk/pull/25450#discussion_r2290345042
PR Review Comment: https://git.openjdk.org/jdk/pull/25450#discussion_r2290345246
PR Review Comment: https://git.openjdk.org/jdk/pull/25450#discussion_r2290344146
PR Review Comment: https://git.openjdk.org/jdk/pull/25450#discussion_r2290344640