RFR: 8367319: Add os interfaces to get machine and container values separately [v5]

David Holmes dholmes at openjdk.org
Mon Dec 8 05:57:07 UTC 2025


On Thu, 4 Dec 2025 10:02:40 GMT, Severin Gehwolf <sgehwolf at openjdk.org> wrote:

>> From my perspective, returning `true` from one of these functions means that the out-parameter is set to a valid, meaningful value and can be used safely by the caller. However, if the function returns `false`, there is no guarantee that the out-parameter is untouched, so relying on its previous value is unsafe. Your example is to me is a bad pattern and is brittle at best.
>> 
>> There is a distinction between `OSContainer::memory_limit_in_bytes` (which always fetches and returns the cgroup, including `value_unlimited` for no limit) and the higher-level `os::Container::memory_limit`, which is about determining whether there is an effective limit at all, and if so, what it is. It doesn't make sense to return `true` if there is no limit, since there wouldn't be a meaningful value to provide. `value_unlimited` is just an internal constant and not helpful for callers.
>> 
>> That said, I can update so we don't pass through the out-parameter so that we don't modify it when returning `false`, to be somewhat more consistent. But I still believe we should return `false` when no limit is set.
>
>> There is a distinction between `OSContainer::memory_limit_in_bytes` (which always fetches and returns the cgroup, including `value_unlimited` for no limit) and the higher-level `os::Container::memory_limit`, which is about determining whether there is an effective limit at all, and if so, what it is. It doesn't make sense to return `true` if there is no limit, since there wouldn't be a meaningful value to provide. `value_unlimited` is just an internal constant and not helpful for callers.
> 
> OK. This should be noted in a comment for `os::Container::xxx` functions.
> 
>> That said, I can update so we don't pass through the out-parameter so that we don't modify it when returning `false`, to be somewhat more consistent.
> 
> I'd prefer that, yes.

> However, if the function returns false, there is no guarantee that the out-parameter is untouched

I think that should be guaranteed. This is our API we can make it as strict as we desire. If I have preset a sentinel value and the op fails then I should be able to rely on my sentinel remaining intact.

> It doesn't make sense to return true if there is no limit

But now you are overloading the use of the return value. It was intended to reflect success or failure, not provide semantic information.  The higher-level function can choose to return true when there is a limit and false when there is not, but it can't also mean "or an error occurred".

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

PR Review Comment: https://git.openjdk.org/jdk/pull/27646#discussion_r2597137251


More information about the hotspot-dev mailing list