RFR: 8321931: memory_swap_current_in_bytes reports 0 as "unlimited"

[Gerard Ziemski]

On Tue, 16 Jan 2024 18:07:25 GMT, Thomas Stuefe <stuefe at openjdk.org> wrote:

>> We make a distinction between 0 for a limit vs non-limit cgroup values.
>> 
>> For a limit-type value, 0 means `unlimited`. For all the others 0 means simply 0.
>> 
>> Output before from `jcmd PID VM.info`:
>> 
>> 
>> container (cgroup) information:
>> container_type: cgroupv2
>> cpu_cpuset_cpus: not supported
>> cpu_memory_nodes: not supported
>> active_processor_count: 8
>> cpu_quota: not supported
>> cpu_period: not supported
>> cpu_shares: not supported
>> memory_limit_in_bytes: unlimited
>> memory_and_swap_limit_in_bytes: unlimited
>> memory_soft_limit_in_bytes: unlimited
>> memory_usage_in_bytes: 11129120 k
>> memory_max_usage_in_bytes: not supported
>> memory_swap_current_in_bytes: unlimited
>> memory_swap_max_limit_in_bytes: unlimited
>> maximum number of tasks: 18963
>> current number of tasks: 33
>> 
>> 
>> Output now from `jcmd PID VM.info`:
>> 
>> 
>> container (cgroup) information:
>> container_type: cgroupv2
>> cpu_cpuset_cpus: not supported
>> cpu_memory_nodes: not supported
>> active_processor_count: 8
>> cpu_quota: not supported
>> cpu_period: not supported
>> cpu_shares: not supported
>> memory_limit_in_bytes: unlimited
>> memory_and_swap_limit_in_bytes: unlimited
>> memory_soft_limit_in_bytes: unlimited
>> memory_usage_in_bytes: 4962584 k
>> memory_max_usage_in_bytes: not supported
>> memory_swap_current_in_bytes: 0
>> memory_swap_max_limit_in_bytes: unlimited
>> maximum number of tasks: 18963
>> current number of tasks: 33
>> 
>> In this example `memory_swap_current_in_bytes` should be printed as equal to 0, not "unlimited".
>
> src/hotspot/os/linux/osContainer_linux.cpp line 142:
> 
>> 140: void OSContainer::print_container_helper(outputStream* st, jlong j, const char* metrics, boolean limit) {
>> 141:   st->print("%s: ", metrics);
>> 142:   if ((j > 0) || ((j == 0) && (limit == false))) {
> 
> Clearer would be a reversal like this:
> 
> if (j == 0 && limit) {
>   "unlimited"
> } else {
>   print
> }

I think I decided to have it this way to draw the attention to the fact that the value `0` for a `non-limit` kind should be treated as numerical value and not a special case.

This provides, in my opinion, a self-documenting code without the need for an explicit comment. That was my rationale at least, but if that doesn't come across, then we can simplify and add an actual comment?

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

PR Review Comment: https://git.openjdk.org/jdk/pull/17314#discussion_r1454159074