[8u] RFR: 8203357: Container Metrics
Andrew Hughes
gnu.andrew at redhat.com
Tue Jul 21 19:38:00 UTC 2020
On 16/07/2020 16:48, Severin Gehwolf wrote:
> Hi,
>
> Please review this OpenJDK 8u backport of the Java Metrics classes
> which are in later JDKs. It is a pre-requisite for JDK-8226575 an
> Oracle JDK parity issue. Implementations of the interface are provided
> for Linux only and, thus, are compiled for Linux only. This backport
> differs from the original JDK 11 change in the following way:
>
> * MetricsTester: Arrays.compare() => Arrays.equals(). Arrays.compare()
> is JDK 9+ only.
> * JDK-8223147: JFR Backport, brought in test library code included in
> the JDK 11 changeset. I've only done the needed adjustments to the
> relevant files in the test libary. Many were already present.
> * For this backport I've not included changes to the Launcher to
> support -XshowSettings:system (as in later JDKs).
>
> Bug: https://bugs.openjdk.java.net/browse/JDK-8203357
> webrev:
> http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8203357/jdk8/01/webrev/
>
> Testing: Included docker tests on Linux x86_64. All pass. Though, they
> are in dire need of reliability improvements which I'll work on as a
> follow-up.
>
> Thanks,
> Severin
>
Due to the differing nature of the two patches, it was necessary to
split them into three; the set of changes applicable to 8u & 11u, the
changes only in 8u and the changes only in 11u.
1. Common changes
diffstat for 11u:
b/src/java.base/linux/classes/jdk/internal/platform/cgroupv1/Metrics.java | 461 +++++++++
b/src/java.base/linux/classes/jdk/internal/platform/cgroupv1/SubSystem.java | 208 ++++
b/src/java.base/share/classes/jdk/internal/platform/Container.java
| 45
b/src/java.base/share/classes/jdk/internal/platform/Metrics.java
| 508 ++++++++++
b/test/jdk/jdk/internal/platform/cgroup/TestCgroupMetrics.java
| 57 +
b/test/jdk/jdk/internal/platform/docker/Dockerfile-BasicTest
| 8
b/test/jdk/jdk/internal/platform/docker/Dockerfile-BasicTest-aarch64
| 8
b/test/jdk/jdk/internal/platform/docker/Dockerfile-BasicTest-ppc64le
| 10
b/test/jdk/jdk/internal/platform/docker/Dockerfile-BasicTest-s390x
| 7
b/test/jdk/jdk/internal/platform/docker/MetricsCpuTester.java
| 178 +++
b/test/jdk/jdk/internal/platform/docker/MetricsMemoryTester.java
| 140 ++
b/test/jdk/jdk/internal/platform/docker/TestDockerCpuMetrics.java
| 167 +++
b/test/jdk/jdk/internal/platform/docker/TestDockerMemoryMetrics.java
| 151 ++
b/test/jdk/jdk/internal/platform/docker/TestSystemMetrics.java
| 61 +
14 files changed, 2009 insertions(+)
diffstat for 8u:
new/src/linux/classes/jdk/internal/platform/cgroupv1/Metrics.java |
461 +++++++++
new/src/linux/classes/jdk/internal/platform/cgroupv1/SubSystem.java |
208 ++++
new/src/share/classes/jdk/internal/platform/Container.java | 40
new/src/share/classes/jdk/internal/platform/Metrics.java |
506 ++++++++++
new/test/jdk/internal/platform/cgroup/TestCgroupMetrics.java |
55 +
new/test/jdk/internal/platform/docker/Dockerfile-BasicTest | 8
new/test/jdk/internal/platform/docker/Dockerfile-BasicTest-aarch64 | 8
new/test/jdk/internal/platform/docker/Dockerfile-BasicTest-ppc64le | 10
new/test/jdk/internal/platform/docker/Dockerfile-BasicTest-s390x | 7
new/test/jdk/internal/platform/docker/MetricsCpuTester.java |
178 +++
new/test/jdk/internal/platform/docker/MetricsMemoryTester.java |
140 ++
new/test/jdk/internal/platform/docker/TestDockerCpuMetrics.java |
164 +++
new/test/jdk/internal/platform/docker/TestDockerMemoryMetrics.java |
143 ++
new/test/jdk/internal/platform/docker/TestSystemMetrics.java |
58 +
14 files changed, 1986 insertions(+)
Comments:
* @author tags are removed from Container.java & Metrics.java. Why? I
can see a case for removing the "@since 11" but not the authorship.
* Removal of test code related to the module system seems fine.
2. 8u only changes
diffstat:
new/make/CompileJavaClasses.gmk | 6
++++++
new/test/lib/jdk/test/lib/containers/cgroup/MetricsTester.java | 8
++++----
2 files changed, 10 insertions(+), 4 deletions(-)
Comments:
* The Makefile changes look right and in line with those for Mac OS & AIX.
* The MetricsTester changes look correct. I can only imagine this
wasn't caught when the test was backported because JDK-8206456 was also
backported and so the length != 0 checks that introduced were never
triggered (as the metrics weren't actually there)
3. 11u only changes
b/src/java.base/share/classes/sun/launcher/LauncherHelper.java
| 109 +
b/src/java.base/share/classes/sun/launcher/resources/launcher.properties | 8
b/test/hotspot/jtreg/runtime/containers/docker/TestCPUAwareness.java
| 4
b/test/hotspot/jtreg/runtime/containers/docker/TestCPUSets.java
| 4
b/test/hotspot/jtreg/runtime/containers/docker/TestMemoryAwareness.java
| 5
b/test/hotspot/jtreg/runtime/containers/docker/TestMisc.java
| 7
b/test/jdk/TEST.ROOT
| 3
b/test/jdk/tools/launcher/Settings.java
| 19
b/test/lib/jdk/test/lib/containers/cgroup/CPUSetsReader.java
| 46
b/test/lib/jdk/test/lib/containers/cgroup/MetricsTester.java
| 589 ++++++++++
b/test/lib/jdk/test/lib/containers/docker/Common.java
| 6
b/test/lib/jdk/test/lib/containers/docker/DockerRunOptions.java
| 7
b/test/lib/jdk/test/lib/containers/docker/DockerTestUtils.java
| 3
13 files changed, 778 insertions(+), 32 deletions(-)
Comments:
* I don't see why the launcher changes are being excluded. Yes, it
would require a CSR [0], but this doesn't seem like a blocker to me.
* All test changes are already present, including file moves, with the
exception of one inapplicable import of CPUSetsReader which is not in a
package in 8u. I'm not sure some of these file moves are correct, and
there seems to be duplication of WhiteBox.java between the HotSpot and
JDK repositories, but that's for another bug.
So, action items are the missing @author tags and launcher support.
[0] https://bugs.openjdk.java.net/browse/JDK-8204107
Thanks,
--
Andrew :)
Senior Free Java Software Engineer
OpenJDK Package Owner
Red Hat, Inc. (http://www.redhat.com)
PGP Key: ed25519/0xCFDA0F9B35964222 (hkp://keys.gnupg.net)
Fingerprint = 5132 579D D154 0ED2 3E04 C5A0 CFDA 0F9B 3596 4222
More information about the jdk8u-dev
mailing list