Dead code in jdk.internal.platform.Metrics?
Severin Gehwolf
sgehwolf at redhat.com
Wed Sep 18 18:05:31 UTC 2019
On Wed, 2019-09-18 at 10:15 -0700, Bob Vandette wrote:
> These internal methods were implemented in order to expose container metrics to
> via a public API. I’ve filed JDK-8203359 and JDK-8199944 to add an MBean
> and JFR events that will expose these values.
>
> JDK-8203359: Java Flight Recorder (JFR) improvements for containers
> JDK-8199944: Add Container MBean to JMX
Sure. Has there been any movement on them?
> There are container tests that verify many of these methods.
Like I said, I've run container tests with the patch below applied and
they seem to pass. So IMO they're not covered by tests. Grepping
through the code doesn't return anything either.
> Can you give me a list of the metrics that are not available under
> cgroupv2?
Not an exhaustive list, but from a cursory initial assessment:
- public long getMemoryMaxUsage();
- public long getKernelMemoryFailCount();
- public long getKernelMemoryMaxUsage();
- public long getKernelMemoryUsage();
- public long getTcpMemoryFailCount();
- public long getTcpMemoryMaxUsage();
- public long getTcpMemoryUsage();
Thanks,
Severin
> 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
> >
More information about the core-libs-dev
mailing list