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

[Stefan Karlsson]

On Thu, 31 Jul 2025 14:37:45 GMT, Anton Artemov <duke at openjdk.org> wrote:

>>> With that said, when looking at this I'm wondering if there is any scenario where we can hit an error when calling the os::physical_memory? I understand that the OS APIs says that we can hit an error, but I wonder if that's only a theoretical issue? If it turns out that the APIs in effect never will fail then we can verify that they succeed during the JVM initialization and then remove the error code from os os::physical_memory() and let it only return a size_t instead. If we do this we can clean away a lot of the error handling code that is added in this PR.
>> 
>> I absolutely agree. Keep it simple and stupid.
>> 
>>> 
>>> The places that I see that could theoretically fail is:
>>> 
>>>     1. On AIX, but there's an `assert(0` in there to show that this really doesn't happen.
>>> 
>>>     2. On Windows if GlobalMemoryStatusEx(&ms) fails. The question is will this call fail for any reason on Windows? Maybe someone from Microsoft could help and tell if there ever is a reason why this code would fail:
>>> 
>>> 
>>> ```
>>>   MEMORYSTATUSEX ms;
>>>   ms.dwLength = sizeof(ms);
>>>   GlobalMemoryStatusEx(&ms);
>>> ```
>> 
>> I don't think this can even have failed, we would have noticed:
>> 
>> - we use `GlobalMemoryStatusEx` to calculate `MaxRAM` (without error checking)
>> - on debug builds, this would return something like "0xcccccccc_cccccccc" on error as size, since that's the default initialization by VisualC++. That's a lot of RAM :-)
>> - we then base our default heap size on that. Reserving that much space for heap would have failed at at least some tests.
>> 
>> In product builds, the result would be random garbage; either 0, which would have failed, too, or some random number with a high probability of being very high, since the probability of random garbage that just happens to look like a small 64-bit number with zeros in the upper word is not that great. So we would have seen at least some errors on Windows in release builds, too.
>
> Okay, makes sense, it looks like a failure of `os::physical_memory()` is something very unlikely to happen. So I made this function `void`, but, to be consistent with the rest, it provides the value by writing to the input parameter.

1) I had hoped for some kind of sanity check in the initialization code that sets up _physical_memory. We can handle that as a separate RFE.

2) I really would prefer if os::physical_memory() returned size_t via the normal means and only use output parameters for the functions that require more than one return value. And on that note ...

3) Do the other functions (free_memory and available_memory) ever fail or could we fix those as well? Windows and AIX are using the same APIs that are used for os::physical_memory. Linux uses sysinfo, which only seems to fail if we pass in an incorrect value. BSD has an assert that would trigger if this fails.

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

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