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

Anton Artemov duke at openjdk.org
Wed Jul 30 14:20:00 UTC 2025


On Tue, 29 Jul 2025 11:32:10 GMT, Stefan Karlsson <stefank at openjdk.org> wrote:

>> Anton Artemov has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 12 commits:
>> 
>>  - 8357086: Fxied return value
>>  - 8357086: Fixed whitespaces
>>  - 8357086: Introduced usage pattern
>>  - 8357086: Fixed typo
>>  - 8357086: Refactored physical_memory in different OS
>>  - 8357086: Small fixes 2
>>  - 8357086: Small fixes 1.
>>  - 8357086: Refactored physical_memory()
>>  - 8357086: Refactored free_swap_space()
>>  - 8357086: Refactored total_swap_space()
>>  - ... and 2 more: https://git.openjdk.org/jdk/compare/75ce44aa...e4698333
>
> src/hotspot/share/compiler/compileBroker.cpp line 1062:
> 
>> 1060:   // Now, we do the more expensive operations.
>> 1061:   size_t free_memory = 0;
>> 1062:   os::free_memory(free_memory);
> 
> I wonder if this should be changed to something like this:
> Suggestion:
> 
>   // Return value ignored - defaulting to 0 on failure.
>   (void)os::free_memory(free_memory);
> 
> to clearly indicate the intent to skip the error check?

This is a valid point, but then we'll need to address it somehow else once a pattern with `[[nodiscard]]` or enum class return value is introduced. Or, simply add an empty clause for failure. For now, I will just put a comment there as suggested.

> src/hotspot/share/prims/whitebox.cpp line 2515:
> 
>> 2513:   LINUX_ONLY(return os::Linux::physical_memory();)
>> 2514:   os::physical_memory(phys_mem);
>> 2515:   return static_cast<jlong>(phys_mem);
> 
> This looks like the JFR code that doesn't want to report the container limits. We should probably have an os:: function that returns this value that both these functions wants. Maybe `os::host_physical_memory`. Something for a future RFE.

Agree.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/25450#discussion_r2242853425
PR Review Comment: https://git.openjdk.org/jdk/pull/25450#discussion_r2242858331


More information about the hotspot-dev mailing list