RFR: 8331560: Refactor Hotspot container detection code so that subsystem delegates to controllers [v7]

Thomas Stuefe stuefe at openjdk.org
Tue Jul 2 06:32:26 UTC 2024


On Mon, 1 Jul 2024 13:56:37 GMT, Severin Gehwolf <sgehwolf at openjdk.org> wrote:

>> Please review this preparatory PR which is an enabler for some bugfix/alignment work (e.g. [JDK-8322420](https://bugs.openjdk.org/browse/JDK-8322420)). The idea is to delegate limit lookup to controllers (`memory`, `cpu`, etc.) from the overarching `CgroupSubsystem` class. This way we can - once the cg type has been detected - "adjust" a controller's path to the limit files when the controller is being created (on init) and then left alone. In this case, the two preparatory entry points are `CgroupUtil::processor_count()` taking a version-agnostic `CgroupCpuController` to do the actual look-up and `CgroupMemoryController::read_memory_limit_in_bytes()` for the same in terms of memory limits.
>> 
>> This enables setting the contoller's path to the interface files (wherever it might be in the hierarchy of it's original cgroup path), look up the limit and "freeze" the path once it found a - lower - limit.
>> 
>> It also ensures that both cgroup versions return `-1` or `-2` (`OSCONTAINER_ERROR`) - in both cases negative - for some notion of unlimited. I.e. it makes the upper bound by the host's physical memory apparent in the version agnostic classes.
>> 
>> Testing:
>> - [x] GHA
>> - [x] gtest:cgroupTest tests
>> - [x] container tests on Linux cgv1 and cgv2 on x86_64
>> 
>> Thoughts?
>
> Severin Gehwolf has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 46 commits:
> 
>  - Fixes after merge
>  - Merge branch 'master' into jdk-8331560-cgroup-controller-delegation
>  - Add comment about un-used-ness
>  - Helper function naming fixes
>  - Remove declaration from sub-classes that are implemented in super class
>  - Use references when passing in readers
>  - Merge branch 'master' into jdk-8331560-cgroup-controller-delegation
>  - Merge branch 'master' into jdk-8331560-cgroup-controller-delegation
>  - Rename log function
>  - Review feedback
>  - ... and 36 more: https://git.openjdk.org/jdk/compare/0a6ffa57...dcd0554b

This has been shaping up nicely. From my point of view, this is good to go. Leave it up to @jdksjolen to add any more suggestions.

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

Marked as reviewed by stuefe (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/19085#pullrequestreview-2152811347


More information about the hotspot-runtime-dev mailing list