On Tue, 9 Feb 2021 13:31:25 GMT, Severin Gehwolf <sgehw...@openjdk.org> wrote:

>> This is an enhancement which solves two issues:
>> 
>> 1. Multiple reads of relevant cgroup interface files. Now interface files 
>> are only read once per file (just like Hotspot).
>> 2. Proxies creation of the impl specific subsystem via `determineType()` as 
>> before, but now reads all relevant interface files: `/proc/cgroups`, 
>> `/proc/self/mountinfo` and `/proc/self/cgroup`. Once read it passes the 
>> parsed information to the impl specific subsystem classes for instantiation. 
>> This allows for more flexibility of testing as interface files can be mocked 
>> and, thus, more cases can be tested that way without having access to these 
>> specific systems. For example, proper regression tests for JDK-8217766 and 
>> JDK-8253435 have been added now with this in place.
>> 
>> * [x] Tested on Linux x86_64 on cgroups v1 and cgroups v2. Container tests 
>> pass.
>
> Severin Gehwolf has updated the pull request with a new target base due to a 
> merge or a rebase. The incremental webrev excludes the unrelated changes 
> brought in by the merge/rebase. The pull request contains seven additional 
> commits since the last revision:
> 
>  - Fix jcheck
>  - Add documentation and reduce code running in the critical section
>  - Add some documentation
>  - Merge branch 'master' into jdk-8254001-enhance-file-parsing-java-metrics
>  - Merge branch 'master' into jdk-8254001-enhance-file-parsing-java-metrics
>  - Merge branch 'master' into jdk-8254001-enhance-file-parsing-java-metrics
>  - 8254001: [Metrics] Enhance parsing of cgroup interface files for version 
> detection

Hi Severin,
Thanks for doing this!  Sorry for taking so long to review this change.  The 
change looks good.  Before pushing it, could you add a comment explaining what 
the code in lines 185-194 of CgroupSubsystemFactory.java is doing?  Also, 
please don't overwrite the fix for JDK-8257746.
Thanks again! Harold

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

Marked as reviewed by hseigel (Reviewer).

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

Reply via email to