On Fri, 25 Aug 2023 10:04:28 GMT, Alan Bateman <al...@openjdk.org> wrote:
> In this case, it seems a bit fragile to do it in CgroupUtil as it should be > allowed to use any of the file system APIs to access cgroups or proc files. In theory, yes. It should be able to use any file system APIs. Practically, it doesn't make a whole lot of difference :-) Going by the last few years this area of the code hasn't had many updates. > Part of me wonders if this would be better handled in their implementation of > jdk.internal.misc.VM or Runtime.maxMemory but maybe you've been there already? Implementing `Runtime.maxMemory` (determine the configured max heap size), usually needs some notion of 'Physical Memory', since the heap size is being based on 'Physical Memory' if not explicitly set. As with HotSpot, that notion of 'Physical Memory' might depend on whether or not you're running in a container (usually with a single process in that container) or not. The GraalVM implementation uses the JDK cgroups code to figure out the 'Physical Memory' in the container case. I don't think there is a way to implement `Runtime.maxMemory` without knowing `Physical Memory'. This isn't a problem in OpenJDK (yet), since HotSpot has its own implementation in its runtime written in C. That has its own set of problems, since we need to update both implementations when bugs come in. In a way, this boils down to ByteBuffers using max heap size for it's default direct memory size determination. I'm not sure doing something else in GraalVM for determining the default direct memory size would be any better. All things considered, breaking the cycle in OpenJDK seems reasonable to me. Would this convince you to accept this patch? ------------- PR Comment: https://git.openjdk.org/jdk/pull/15416#issuecomment-1693364297
