RFR(s): 8250627: Use -XX:+/-UseContainerSupport for enabling/disabling Java container metrics

[David Holmes]

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
>>>
>