RFR: JDK-8287011: Container information could be improved [v3]

[Severin Gehwolf]

On Fri, 3 Jun 2022 14:05:27 GMT, Matthias Baesken <mbaesken at openjdk.org> wrote:

>> The change adds some potentially interesting cgroup v1 and v2 metrics to hs_err / hs_info files.
>
> Matthias Baesken has updated the pull request incrementally with one additional commit since the last revision:
> 
>   handle new cgroupv1 and v2 values in TestMisc

Please consider pushing implementation details of the version specific printing to the implementation classes.

src/hotspot/os/linux/cgroupSubsystem_linux.hpp line 261:

> 259: 
> 260:     virtual jlong memory_swap_current_in_bytes() = 0;
> 261:     virtual jlong memory_swap_max_limit_in_bytes() = 0;

I'm really not fond of this increased API surface for properties which are only supported on one version, but not the other. The idea would be to use those for printing in `os_linux`, right? Could we not move the implementation detail to the version specific classes? I.e. define `virtual void print_version_specific_info(outputStream* st) = 0` here and in `osContainer_linux.hpp` then only call `OSContainer::print_version_specific_info(st)` in `os_linux` and be done with it? This would avoid the empty stub implementations in the version specific classes...

src/hotspot/os/linux/cgroupV2Subsystem_linux.cpp line 250:

> 248: }
> 249: 
> 250: 

These should really not be needed if we make the `print_version_specific_info(...)` function implementation defined.

src/hotspot/os/linux/os_linux.cpp line 2271:

> 2269:   // the kmem (kernel memory) values are only available in cgroupv1
> 2270:   if (p_ct != NULL && strcmp(p_ct, "cgroupv1") == 0) {
> 2271:     print_container_helper(st, OSContainer::kernel_memory_usage_in_bytes(), "kernel_memory_usage_in_bytes");

We wouldn't need the extra version checking gymnastics if we'd call an `OSContainer::print_version_specific_info(st);` here instead.

test/hotspot/jtreg/containers/docker/TestMisc.java line 127:

> 125:         }
> 126:         String str = out.getOutput();
> 127:         if (str.contains("cgroupv1")) {

An alternative would be to pass a `Metrics` instance (`Metrics.systemMetrics()`) to `checkContainerInfo` and use:
`"cgroupv1".equals(m.getProvider())` here instead.

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

Changes requested by sgehwolf (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/8991