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

Johan Sjölen jsjolen at openjdk.org
Thu Jun 13 10:42:16 UTC 2024


On Wed, 12 Jun 2024 19:13:24 GMT, Thomas Stuefe <stuefe at openjdk.org> wrote:

>> Severin Gehwolf has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 30 commits:
>> 
>>  - Review feedback
>>  - Clean up
>>  - Appropriately handle version specific printing
>>  - Fix inheritance hierarchy
>>  - Fixup after merge
>>  - Merge branch 'master' into jdk-8331560-cgroup-controller-delegation
>>  - Resolve ambiguity with static_cast
>>  - Remove limit_from_str from util class
>>  - Merge branch 'jdk-8302744-cleanup-getcontainer-info' into jdk-8331560-cgroup-controller-delegation
>>  - Add proper comments for parsing utility functions
>>  - ... and 20 more: https://git.openjdk.org/jdk/compare/9b64ece5...e177a086
>
> src/hotspot/os/linux/cgroupV1Subsystem_linux.hpp line 52:
> 
>> 50:     char *subsystem_path() { return _path; }
>> 51: };
>> 52: 
> 
> Drive by comment. I don't understand the aim behind the complexity, but multiple inheritance, even with pure abstract side classes, is rarely a good idea. It is also against the hotspot style guide:
> 
> 
> Restrict inheritance to the "is-a" case; use composition rather than
> non-is-a related inheritance.
> 
> Avoid multiple inheritance.  Never use virtual inheritance.
> 
> 
> Most cases I have seen of multiple inheritance are driven by the wish to reuse code, but the added complexity and confusion (e.g. typical C++ IDEs cannot follow call chains) makes life for maintainers actually harder. Composition is usually a better way to do things.

This kind of multiple-inheritance isn't a big deal, it's the same as having multiple `implements` classes in Java. @jerboaa , is it possible to avoid the multiple inheritance by stuffing a `CgroupV1Controller` into the `CgroupV1MemoryController` and so on? Or at least *only* inheriting from classes which are abstract and **only** exposes a set of `virtual = 0;` type methods.

Basically this:

```c++
class CgroupV1MemoryController {
  CgroupV1Controller _parent;
};

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

PR Review Comment: https://git.openjdk.org/jdk/pull/19085#discussion_r1637968872


More information about the hotspot-runtime-dev mailing list