[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