RFR: 8302744: Refactor Hotspot container detection code

Johan Sjölen jsjolen at openjdk.org
Fri May 3 22:05:11 UTC 2024


On Thu, 2 May 2024 12:36:11 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?

Hi @jerboaa ,

I'm happy to see effort being put towards  a cleanup of this code. I'll help out with reviewing next week.

src/hotspot/os/linux/cgroupSubsystem_linux.hpp line 193:

> 191: PRAGMA_DIAG_PUSH
> 192: PRAGMA_FORMAT_NONLITERAL_IGNORED
> 193: template <typename T> int cg_file_multi_line_ctrl(CgroupController* c,

This always bothered me: If the first argument is a pointer to a `CgroupController`, then why isn't this just a method for that object? No need to for the `c == nullptr` check then either.

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

PR Review: https://git.openjdk.org/jdk/pull/19060#pullrequestreview-2039048294
PR Review Comment: https://git.openjdk.org/jdk/pull/19060#discussion_r1589757769


More information about the hotspot-runtime-dev mailing list