RFR: 8357086: os::xxx functions returning memory size should return size_t [v13]
Stefan Karlsson
stefank at openjdk.org
Thu Jul 31 09:46:02 UTC 2025
On Thu, 31 Jul 2025 08:56:48 GMT, Anton Artemov <duke at openjdk.org> wrote:
> the 1st summand (in braces) is returned by `OSContainer::memory_and_swap_limit_in_bytes()`, the 2nd by `OSContainer::memory_limit_in_bytes()`. The problem is that `swap_limit` can be unbounded (represented by -1 in the container code). So we end up in returning `static_cast<size_t>(-1)`, which would be 2^64-1 on 64bit platform and is the same as `std::numeric_limits<size_t>::max()`.
>
> In my opinion, it is a good representation of the _unbounded_ value.
The error code -1 happens to be interpretable as a large number if it is converted to a unsigned value, and one can argue that this large number is an OK value to represent an unbound value. Personally, I think that is more accidental than the intended behavior of this code. This is an obscure detail in the code, and given that it isn't immediately obvious for the reader it also makes it a risk that people will not understand this if they make future changes to the code.
And even we accept that -1 can be used as the unbound value, then I think it is wrong to use the unbound value in this code. If the container says that the swap space is unbound can't the underlying host still return a sensible value in si.totalswap? If this is the case, then an error in the container code will cause os::total_swap_space to return a way higher swap value than what the host is configured to have.
With all this said, I think the above code should check if memory_and_swap_limit_in_bytes returns -1, just like we do in os::free_swap_space().
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/25450#discussion_r2244885615
More information about the hotspot-dev
mailing list