[8u] RFR: 8203357: Container Metrics
Severin Gehwolf
sgehwolf at redhat.com
Thu Jul 30 09:17:18 UTC 2020
Hi Andrew,
Thanks for the review!
On Tue, 2020-07-21 at 20:38 +0100, Andrew Hughes wrote:
>
> 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
> >
[...]
>
> 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.
[...]
> 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)
It appears MetricsTester was never run, thus compiled. So I think this
never worked in 8u. Now code uses it and the Arrays.compare() JDK 9+
feature needed to be reworked.
[...]
> So, action items are the missing @author tags and launcher support.
New webrev:
http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8203357/jdk8/02/webrev/
Thoughts?
Thanks,
Severin
More information about the jdk8u-dev
mailing list