RFR: 8331560: Refactor Hotspot container detection code so that subsystem delegates to controllers [v3]
Severin Gehwolf
sgehwolf at openjdk.org
Thu Jun 13 15:46:14 UTC 2024
On Thu, 13 Jun 2024 10:38:01 GMT, Johan Sjölen <jsjolen at openjdk.org> wrote:
>> 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;
>> };
>
> Reading this code a bit more, seems like `CgroupController` just exposes a virtual function `subsystem_path`. But this function is only ever called from within `CgroupController`, and we never seem to want to handle a Cgroup controller in a generic way anyway. We can just make the subsystem_path be an argument to all of the functions which require it in `CgroupController` and make that class `AllStatic` and have no one inherit from it.
>
> I also find the general usage of `malloc` concerning. looking at the class `CgroupV1Controller`, this doesn't have a corresponding destructor even though it seems like it owns the memory which it has pointers to. This can just hold the memory directly, no pointers necessary.
Basically a `CgroupController`'s job is to read from the interface file. In order to do so it needs a base path to the interface files, which is determined elsewhere.
I'll rework the patch to get rid of the multiple-inheritance.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/19085#discussion_r1638439423
More information about the hotspot-runtime-dev
mailing list