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