[8u] RFR: 8226575: OperatingSystemMXBean should be made container aware

Severin Gehwolf sgehwolf at redhat.com
Mon Aug 24 12:29:23 UTC 2020


On Fri, 2020-08-21 at 19:37 +0100, Andrew Hughes wrote:
> 
> > > There is no Windows file in the 11u version. Why is it needed in 8u?
> > > In both, OperatingSystemImpl.java seems to be under
> > > src/jdk.management/unix/classes or src/solaris/classes/sun/management,
> > > rather than share/classes.
> > 
> > Good point. There is a Windows specific OperatingSystemImpl.java with
> > references to native methods. I missed that. Note to self: No need to
> > change the native method impls as they've not changed
> > in src/windows/classes/sun/management/OperatingSystemImpl.java either.
> > Reverted in the updated webrev.
> 
> Thanks. It took me a while to spot it as well. At first, I thought Windows
> was picking it up from one common shared Java file like the others, but
> then I looked at the paths again and spotted the unix/solaris usage.
> 
> > Latest webrev here:
> > http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8226575/jdk8/02/webrev/
> > 
> 
> Thanks. JDK side looks good now.

Thanks again for the review!

> I'll wait for your response on
> whether to do the other change first for the HotSpot side.

Please, lets not delay this any longer for a nice-to-have test cleanup.

> > > Your description of the Subsystem.java changes is quite terse.
> > > Can you explain in more detail why the readMatchingLines change
> > > is not needed?
> > 
> > OpenJDK 8u doesn't have this bug which introduces readMatchingLines()
> > methods:
> > 
> > 8217338: [Containers] Improve systemd slice memory limit support
> > 
> > Therefore, hunks which adapted those methods, or rather, refactored
> > them, aren't needed in this patch. Does that make sense?
> > 
> 
> It does. I think I pretty much got there from the shorter comment,
> but my brain wasn't sure enough that it had parsed it correctly.
> 
> > > OperatingSystemImpl.java & OperatingSystemMXBean.java look fine.
> > 
> > Tested builds on Windows, AIX, Solaris and Linux with this updated
> > webrev.
> > 
> 
> You have access to AIX & Solaris builds? That might be useful for
> other patches.

Yes, I only realized this recently. With help from our friends at
Eclipse Adoptium (AdoptOpenJDK).

> Out of interest, did the original pass on Windows?

I haven't tried, so I don't know. Probably not.

Thanks,
Severin



More information about the jdk8u-dev mailing list