RFR: 8302744: Refactor Hotspot container detection code [v2]
Severin Gehwolf
sgehwolf at openjdk.org
Tue May 21 14:10:06 UTC 2024
On Tue, 21 May 2024 13:58:22 GMT, Severin Gehwolf <sgehwolf at openjdk.org> wrote:
>> Please review this container detection code refactoring in hotspot. The main point of this is to
>>
>> - get rid of the `GET_CONTAINER_INFO` macros which hide too many things under the hood
>> - prevent refactoring of the code (since `GET_CONTAINER_INFO` macros short-return and are therefore not portable; at least not without some risk)
>> - make the code easier to understand
>> - allow for better testing via `gtest`
>> - separate multi-line parsing from single line parsing for clarity.
>>
>> Testing:
>> - [x] GHA
>> - [x] `gtest:cgroupTest` tests
>> - [x] Container tests on Linux with cgroup v1 (legacy) and cgroup v2. All pass.
>>
>> Thoughts?
>
> 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 ten additional commits since the last revision:
>
> - Fix TestMemoryAwareness for cgroup v2
>
> It only reads the swap value, thus doesn't print:
> "Memory and Swap Limit is" but rather prints:
> "Swap Limit is". This is fine, since for cg v2
> the memory limit and the swap limits are in different
> files.
> - Fix whitespace
> - Handle cpu quota for cgroups v1 specially
> - Try a different approach.
>
> Hybrid MACRO usage and locked down reading primitives.
> - Add key-value read function
> - Add basic testing for read functions
> - Add generic julong/char* read functions
> - Merge branch 'master' into jdk-8302744-cleanup-getcontainer-info
> - 8302744: Refactor Hotspot container detection code
test/hotspot/jtreg/containers/docker/TestMemoryAwareness.java line 204:
> 202: // Either for cgroup v2: a_2) 0, or b_2) -2 on systems with swapaccount=0
> 203: .shouldMatch("(Memory and )?Swap Limit is:.*(" + expectedTraceValue + "|-2|0)")
> 204: .shouldNotMatch("(Memory and )?Swap Limit is:.*" + neg2InUnsignedLong);
This is a side-effect of reading the swap limit on cg v2 which only prints `Swap Limit is:` (fixing a FIXME along the way in the code), since it's only reading the swap value in that case. Note that for cg v2 the interface files for swap limit and memory limit are different files and for cg v2, only the swap limit is there (contrary to the cg v1 interface file which contains the memory and swap limit). Therefore I've done an optional match in the test.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/19060#discussion_r1608402043
More information about the hotspot-runtime-dev
mailing list