Hi Severin,
On 28/07/2020 11:23 pm, Severin Gehwolf wrote:
Hi David,
On Tue, 2020-07-28 at 21:17 +1000, David Holmes wrote:
Hi Severin,
On 28/07/2020 6:27 pm, Severin Gehwolf wrote:
Hi,
Please review this patch which makes the Java container metrics adhere
to -XX:+/-UseContainerSupport so they can be disabled if heuristics
turn out to be wrong. The approach taken is to use JNI and call into
the JVM in order to determine the setting of UseContainerSupport before
Metrics are being instantiated.
The intention is for this patch to be backported to older JDKs so as to
provide a means to disable container metrics should things go wrong
with backports of the likes of JDK-8226575.
Bug: https://bugs.openjdk.java.net/browse/JDK-8250627
webrev: https://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8250627/01/webrev/
Seems quite simple and clean.
One query though, I'm not clear on who the expected caller of
Metrics.getInstance() is? (Coming from the perspective of "might we want
to cache the fact container support is not enabled?".)
I know of two uses so far:
1) Launcher (-XshowSettings:system):
http://hg.openjdk.java.net/jdk/jdk/file/89fe9e02a522/src/java.base/share/classes/sun/launcher/LauncherHelper.java#l318
2) OperatingSystemMXBean:
http://hg.openjdk.java.net/jdk/jdk/file/89fe9e02a522/src/jdk.management/unix/classes/com/sun/management/internal/OperatingSystemImpl.java#l48
Both uses seem OK as is. Is it worth caching something here?
No that looks fine. I didn't find them because of the reflective
invocation in Metrics.systemMetrics().
Also note that we no longer update JVM_INTERFACE_VERSION (last update
was JDK 13) - it is meaningless now the JDK and hotspot are in sync
version wise.
OK. Does that mean I should revert the version increment, then?
I would leave it unchanged, yes.
Thanks,
David
-----
Thanks,
Severin
Thanks,
David
Testing: New container test. Existing container tests. jdk/submit.
tier1 on Linux x86_64.
Thoughts?
Thanks,
Severin