On Fri, 25 Aug 2023 10:04:28 GMT, Alan Bateman <al...@openjdk.org> wrote:
>>> Something fishy here, is this working around a bug in GraaVM native image >>> or why does this change need to be in JDK? >> >> I've now realized that the bug had an incorrect statement in the >> description. The cycle happens due to the `Runtime.getRuntime().maxMemory()` >> implementation in GraalVM to use JDK `Metrics`, since the `ByteBuffer` [code >> relies on the `Runtime.getRuntime().maxMemory()` >> API](https://github.com/openjdk/jdk/blob/76b9011c9ecb8c0c713a58d034f281ba70d65d4e/src/java.base/share/classes/jdk/internal/misc/VM.java#L261). >> The GraalVM impl to use the JDK Metrics seems a reasonable thing to do, no? >> >> With that said, it's seems a rather uncontroversial change with very limited >> scope. Do you see anything problematic in this patch? Happy to revise if you >> think there are some no-no's :) > >> I've now realized that the bug had an incorrect statement in the >> description. The cycle happens due to the `Runtime.getRuntime().maxMemory()` >> implementation in GraalVM to use JDK `Metrics`, since the `ByteBuffer` [code >> relies on the `Runtime.getRuntime().maxMemory()` >> API](https://github.com/openjdk/jdk/blob/76b9011c9ecb8c0c713a58d034f281ba70d65d4e/src/java.base/share/classes/jdk/internal/misc/VM.java#L261). >> The GraalVM impl to use the JDK Metrics seems a reasonable thing to do, no? >> >> With that said, it's seems a rather uncontroversial change with very limited >> scope. Do you see anything problematic in this patch? Happy to revise if you >> think there are some no-no's :) > > Recursive initialization issues can tricky and often it comes down to > deciding where to break the cycle. 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. 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? > @AlanBateman Is there anything else you need me to do? If so, please let me > know. Thanks! I don't think the JDK is the right place to workaround this issue. Also, we really need to get back re-implementing FileInputStream and friends on the new API, in which case the original issue will come back again. So I think it would be better to see how this can be fixed in the native image code. ------------- PR Comment: https://git.openjdk.org/jdk/pull/15416#issuecomment-1695902738
