Dead code in jdk.internal.platform.Metrics?

[Bob Vandette]

It appears that many of the metrics we make available are in cgroupv2 although they
are located in different files.  The TCP memory allocations many not be split out.  In the
case of V2 you could just return the not available error return or we could talk about 
removing this metric.

https://www.kernel.org/doc/html/latest/admin-guide/cgroup-v2.html

Here are where most of the Metrics tests are implemented:

{jdk14}/open/test/lib/jdk/test/lib/containers/cgroup/MetricsTester.java

I’m at Code One this week and will take a look at your v2 webrev when I get back to the office.
Do you handle the case where there are v1 and v2 subsystems enabled at the same time?


Bob.


> On Sep 18, 2019, at 10:02 AM, Severin Gehwolf <sgehwolf at redhat.com> wrote:
> 
> Hi Bob,
> 
> As you probably know, I'm looking at cgroups v2 support[0] in OpenJDK.
> When looking at Metrics.java[1] I see that many methods aren't used
> anywhere. Neither in tests nor in actual code. It looks like as if the
> interface has been modelled along the lines of the cgroups v1
> implementation. These methods are:
> 
> -    public long getCpuUsage();
> -    public long[] getPerCpuUsage();
> -    public long getCpuUserUsage();
> -    public long getCpuSystemUsage();
> -    public double getCpuSetMemoryPressure();
> -    public long getMemoryMaxUsage();
> -    public long getMemoryUsage();
> -    public long getKernelMemoryFailCount();
> -    public long getKernelMemoryMaxUsage();
> -    public long getKernelMemoryUsage();
> -    public long getTcpMemoryFailCount();
> -    public long getTcpMemoryMaxUsage();
> -    public long getTcpMemoryUsage();
> -    public long getBlkIOServiceCount();
> -    public long getBlkIOServiced();
> 
> They are essentially dead code. Note that not all of them would have an
> implementation in cgroups v2. With that in mind, does it actually make
> sense for the Metrics interface to define those? We could keep the
> implementations for cgroup v1 Metrics, but perhaps remove them from the
> interface? It seems the current Metrics interface defines too many
> methods for no good reason. Am I missing something? Thoughts?
> 
> From the looks of it it wouldn't even require a CSR as
> jdk.internal.platform isn't an exported package.
> 
> Here is an example patch removing them, and jdk docker container tests
> still pass:
> http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8230305/remove_metrics_interface_methods.patch
> 
> Thanks,
> Severin
> 
> [0] https://bugs.openjdk.java.net/browse/JDK-8230305
> [1] http://hg.openjdk.java.net/jdk/jdk/file/377f47ccc20b/src/java.base/share/classes/jdk/internal/platform/Metrics.java
>