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

[Thomas Stuefe]

On Thu, 29 May 2025 06:37:36 GMT, David Holmes <dholmes at openjdk.org> wrote:

> > I talked to Anton offline about the ptrdiff_t. That type has the correct signedness and number of bits on all our platforms, but to me that type carries a semantic meaning about what pointer diffs / indices. Because of that I found it inappropriate to use that type.
> 
> Those were my thoughts as well when I saw that change made. I don't have a good answer either. It is not at all clear how/why the swap functions were allowed to report errors when none of the others do. It is not documented in os.hpp nor required by the `OperatingSystemMXBean` API!
> 

Even disregarding Linux container case, looking at `available_memory()`, `total_swap_space()` and `free_swap_space()` I see that error handling is inconsistent. At the moment:

- in Windows, we call `GlobalMemoryStatusEx` and then ignore any errors, potentially returning whatever garbage had been in the `MEMORYSTATUSEX` structures when we called the function.

- on AIX, we use the libperfstat for all three. If it fails, we return UINT64_MAX for error in `available_memory()`. `total_swap_space()` and `free_swap_space()` both return -1.

- on BSD, we always return physical memory / 2 for `available_memory()` (?!) Feels similar to the logic of heap size default?

- on Mac, we use host_statistics64() for `available_memory()`, but return physical memory / 2 in case of error (?!).  Both swap functions return -1.

- on bare metal Linux, 
  - `available_memory()` attempts to read /proc/meminfo; failing that, we return - eventually - free memory info from `sysinfo(2)`; that can also fail, but we ignore that. This may be fine; the only documented way sysinfo can fail is via programmer error.
  - `total_swap_space()` and `free_swap_space()` both read `sysinfo(2)` too, but now they we do not ignore the error but return -1 instead. Inconsistent with available_memory()

This may have been more effort than I envisioned when creating this issue, sorry for that. I would love it if we could unify these behaviours.

I dislike that the swap APIs return signed values. This cuts off half of the value range and feels unnatural and out of sync with the other memory APIs. Admittedly, a machine with more than half its address range as swap space is unlikely (BTW, interesting question is 32-bit: e.g. on Windows, with address space extension, we have 3GB user address space; could we have >2GB swap space? Maybe not).

What we could do:
A) return (size_t)-1. That leaves almost the full value range and excludes a value that will never appear in earnest. We could codify that as a handy constant (similar to MAP_FAILED on Posix)
B) return 0. Similar reasoning to above.
C) return a "sensible default" like some APIs do today.
D) a separate error reporting variable. It feels a bit overengineered, though.
E) swallow errors in release, assert in debug.

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

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