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

Stefan Karlsson stefank at openjdk.org
Wed Jul 30 14:40:59 UTC 2025


On Wed, 30 Jul 2025 14:02:05 GMT, Anton Artemov <duke at openjdk.org> wrote:

>> src/hotspot/share/jfr/jni/jfrJniMethod.cpp line 406:
>> 
>>> 404: 
>>> 405: JVM_ENTRY_NO_ENV(jlong, jfr_host_total_memory(JNIEnv* env, jclass jvm))
>>> 406:   size_t phys_mem = 0;
>> 
>> This can be moved below where it is used. I would probably also be good to explicitly show that the return value is skipped.
>> 
>> With that said, now I've seen this pattern a number of times and think that it might be cleaner to have a function named `os::physical_memory_or_0()` (just like we have a bunch of "_or_null" functions) that returns 0 on failure. The code could then be:
>> 
>> #ifdef LINUX
>>   // We want the host memory, not the container limit.
>>   // os::physical_memory() would return the container limit.
>>   return os::Linux::physical_memory();
>> #else
>>   return os::physical_memory_or_0();
>> #endif
>> 
>> We can think about that proposal as a follow-up RFE.
>
> I do not agree that it is clearer to have a function named   `os::physical_memory_or_0()`, because 0 does not necessarily mean a failure. It may well be a valid value for something on a system level. Not even touching various posix stuff returning 0 in case of success.

I think there's a disconnect in our discussion here. My point is that we "laundry" whatever system level error you can get and instead return 0 whenever we can't figure out the amount of physical memory. The thinking is that surely 0 isn't a valid amount of physical memory, so lets have a convenient function that that is easy to use and returns 0 on failure? And the point is that let's use that instead of the handful places that perform the error handling just to convert it to 0 anyways.

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

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


More information about the hotspot-dev mailing list