RFR: 8357086: os::xxx functions returning memory size should return size_t [v11]
Anton Artemov
duke at openjdk.org
Tue Jun 17 09:18:49 UTC 2025
On Fri, 13 Jun 2025 10:14:22 GMT, Anton Artemov <duke at openjdk.org> wrote:
>> Hi,
>>
>> in this PR the output value type for functions which return memory are changed, namely:
>>
>>
>> static julong available_memory(); --> static MemRes available_memory();
>> static julong used_memory(); --> static MemRes used_memory();
>> static julong free_memory(); --> static MemRes free_memory();
>> static jlong total_swap_space(); --> static MemRes total_swap_space();
>> static jlong free_swap_space(); --> static MemRes free_swap_space();
>> static julong physical_memory(); --> static MemRes physical_memory();
>>
>>
>> `MemRes` is a struct containing a pair of values, `size_t value` to carry the return value, `int error` to carry the error if any. Currently, in case of error the latter is set to -1.
>>
>> The changes are done so that the other parts of the code have minimal impact.
>> Tested in GHA and Tiers 1-4.
>
> Anton Artemov has updated the pull request incrementally with one additional commit since the last revision:
>
> 8357086: Added default val to constructor of MemRes.
Okay, let me summarize the findings:
1) One needs to be consistent with respect to the type of the return value, of `os::xxx()` and having `size_t` in all methods is something everyone agrees on.
2) There is a consensus that errors should be reported and handled properly, not ignored.
3) There is only one type of error.
4) The usage pattern should make it difficult for the user to ignore the error if it is reported.
Given the above, I think the optimal solution is to have a boolean as a return type to indicate the error (true for no error, false for error), and the actual memory value as an in-parameter transferred by reference or pointer. The usage pattern may be enforced by `nodiscard` attribute, but it available from C++17 only. For now, one can just add if/else statements, and add he attribute later after upgrade to C++17.
-------------
PR Comment: https://git.openjdk.org/jdk/pull/25450#issuecomment-2979590415
More information about the hotspot-dev
mailing list