RFR: 8254001: [Metrics] Enhance parsing of cgroup interface files for version detection

Severin Gehwolf sgehwolf at openjdk.java.net
Tue Feb 9 13:45:39 UTC 2021


On Mon, 1 Feb 2021 20:26:54 GMT, Andrew John Hughes <andrew at openjdk.org> wrote:

> > Anybody willing to review this?
> 
> I can have a go.
> 
> I have two main concerns:
> 
>     1. There seems to be little documentation on the new additions. I'm particularly concerned about things like CgroupV1Subsystem.java where a big chunk of documentation on the formats being read is removed and I don't see it being moved elsewhere. Documentation on the new getInstance methods would be helpful too.

The updated patch now includes some more documentation. The reason why I've removed some of it was because the logic changed and it was outdated.

>     2. With the new volatile fields, I think it would be better if the lock being held to initialise them was only held when necessary to perform the assignment. Holding it while performing an extensive process of parsing multiple files that could potentially fail seems a little dangerous. i.e. something like:
>        if (INSTANCE == null) { CGroupV1Subsystem tmp = initSubSystem(infos); synchronized (CgroupV1Subsystem.class) { if (INSTANCE == null) { INSTANCE = tmp; } } }

I've changed this as suggested, but keep in mind no parsing of multiple files happens after this patch. It happens in CgroupSubsystemFactory.

Thanks,
Severin

-------------

PR: https://git.openjdk.java.net/jdk/pull/1393


More information about the core-libs-dev mailing list