RFR: 8226575: OperatingSystemMXBean should be made container aware

[Severin Gehwolf]

Hi Andrew,

Note that we should first get the CSR and - what the CSR should entail
- in order before we can pursue this further. I'll help you with that.
More below.

On Thu, 2019-07-18 at 16:28 -0400, Andrew Azores wrote:
> Hi Severin,
> 
> On Thu, 2019-07-18 at 18:37 +0200, Severin Gehwolf wrote:
> > Hi Andrew,
> > 
> > src/jdk.management/share/classes/module-info.java
> > 
> > I don't think you need to explicitly require java.base. Everything
> > depends on java.base. Is there a specific reason why that was needed?
> > 
> 
> Ah, you're right that that's not needed. I figured java.base should be
> implicitly depended upon, but the first run through failed due to
> module visibility. The actual fix needed was the other module-info
> change in the patch, ie:
> 
> +    exports jdk.internal.platform to
> +        jdk.management;
> 
> in the java.base module-info. I've gone ahead and removed the
> unnecessary addition.

OK, cool.

> > I've noticed that this test fails with your patch:
> > 
> > FAILED:
> > com/sun/management/OperatingSystemMXBean/GetSystemCpuLoad.java
> > 
> > Fails with:
> > java.lang.RuntimeException: getSystemCpuLoad() returns
> > 173995.48501979 which is not in the [0.0,1.0] interval
> > 	at GetSystemCpuLoad.main(GetSystemCpuLoad.java:43)
> 
> What did you run to get this test to execute? I hadn't hit that failure
> in running `make test TEST=tier1`, but I can verify the failure if I
> run specifically that class, so I'm unsure what larger suite or tier
> it's included in.

I've run:
$ jtreg -verbose:summary -jdk:./build/linux-x86_64-server-release/images/jdk test/jdk/com/sun/management/OperatingSystemMXBean test/jdk/com/sun/management/UnixOperatingSystemMXBean test/jdk/java/lang/management/OperatingSystemMXBean

> I'll need to go back and rethink how this system load calculation can
> be done.

Sure.

> > This makes me wonder what the effect of this patch is on Linux
> > *outside* a container. Have you verified that Metric values
> > correspond
> > to what whas previously returned via native methods? Could you
> > verify,
> > please and tell us the before/after values? Thanks!
> 
> Other than the system load, the other memory-related values appear to
> be correct both on bare metal and in Docker. Manually verifying the
> tests in com/sun/management/OperatingSystemMXBean with the "trace"
> argument and the patch applied (system load change removed), the values
> printed match my host machine's /proc/meminfo. Running
> jtreg:test/hotspot/jtreg/containers/docker tests also shows that the
> Docker tests succeed and the reported values within a container are as
> expected as well.

OK.

> I can't see that there are any tests to verify the Metrics
> implementation itself. Am I just missing them or have none been
> written?

There is MetricsTester.java in the test library which is beeing used by
TestCgroupMetrics.java and TestSystemMetrics.java

> > I wonder if we should split the OperatingSystemMXBean tests into it's
> > own (and not piggy-back on
> > TestCPUAwareness.java/TestMemoryAwareness.java). Have you considered
> > that?
> > 
> 
> Sure, I could do that for the next review round.

Great, thanks!

Cheers,
Severin