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!

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

PR Comment: https://git.openjdk.org/jdk/pull/15416#issuecomment-1695824275

Reply via email to