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

[Kim Barrett]

On Thu, 12 Jun 2025 14:01:14 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 val` to carry the return value, `int err` 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 with a new target base due to a merge or a rebase. The pull request now contains 17 commits:
> 
>  - 8357086: Fixed merge conflict.
>  - 8357086: Changed returm type to struct.
>  - 8357086: Return size_t from swap mem funcs, added checks.
>  - 8357086: Added missed casts.
>  - 8357086: Changed return type for total_swap_space and free_swap_space to ssize_t
>  - Merge remote-tracking branch 'origin/master' into JDK-8357086_size_t_memfuncs
>  - 8357086: Fixed spaces in formatting in gc-related code.
>  - 8357086: Fixed formatting.
>  - 8357086: Addressed reviewer's comments.
>  - 8357086: More work.
>  - ... and 7 more: https://git.openjdk.org/jdk/compare/e18277b4...dd9275ca

> FWIW, I think that the `ssize_t` was a good first step and the `MemRes` was an experiment that would be interesting to see if how that panned out, but I didn't expect to see it in this PR. It tried to convey that in an earlier comment. I'm leaving it up to the other involved reviewers to decide if we should go with the MemRes change in this PR.
> 
> About the MemRes change. I think adding the MemRes instance and then initializing it later makes the code messier:
> [...] 
> It would be nice if the code could be something like this:
>  [...]
> I think you could even write this if MemRes didn't have any constructors (or maybe it can with constructors?):
> [...]

I agree with @stefank.

Also, allocating the MemRes and filling it in later depends on NRVO for
optimization (though it doesn't matter much here), which is optional and
sometimes disabled by even fairly innocuous code, and remains so even with
C++17. Constructing in the return will benefit from C++17 guaranteed copy
elision.

And yes, the brace initializion example works - C++14 6.3.3. It would
currently be pretty unusual in HotSpot - I don't know what folks might think
about that syntax.

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

PR Comment: https://git.openjdk.org/jdk/pull/25450#issuecomment-2967724516