clintropolis commented on a change in pull request #7606: Set direct memory if
unable to detect JVM config
URL: https://github.com/apache/incubator-druid/pull/7606#discussion_r282326369
##########
File path:
processing/src/main/java/org/apache/druid/query/DruidProcessingConfig.java
##########
@@ -52,7 +52,22 @@ public int intermediateComputeSizeBytes()
return computedBufferSizeBytes.get();
}
- long directSizeBytes =
JvmUtils.getRuntimeInfo().getDirectMemorySizeBytes();
+ long directSizeBytes;
+ try {
+ directSizeBytes = JvmUtils.getRuntimeInfo().getDirectMemorySizeBytes();
Review comment:
I haven't been following the jdk9+ compat PRs too closely so apologies if
this has been discussed elsewhere, but if we aren't afraid of getting a bit
dirty there appear to be at least 2 ways we could source this information which
I tested up to jdk11 (didn't have 12 handy).
[This stuff is still in the jdk, at least as of 12, but it's now in
`jdk.internal.misc.VM`](https://github.com/AdoptOpenJDK/openjdk-jdk12u/blob/master/src/java.base/share/classes/jdk/internal/misc/VM.java#L130).
I imagine the java people would tell us that neither of the ways I could get
this information are legit to use.. but they probably would've said that about
using `sun.misc.VM` in the first place.
The first and less intrusive is to reflect it out of `java.nio.Bits`, which
stores the value it gets from `jdk.internal.misc.VM` in a static field which we
could grab like this:
```
Class<?> bitsClass = Class.forName("java.nio.Bits");
Field maxMemField = bitsClass.getDeclaredField("MAX_MEMORY");
maxMemField.setAccessible(true);
long maxMem = (Long) maxMemField.get(null);
```
However, this will complain loudly on stderr with something like:
```
WARNING: An illegal reflective access operation has occurred
WARNING: Illegal reflective access by me.clintropolis.sandbox.mem.Main
(file:/Users/clint/workspace/clintropolis/sandbox/target/classes/) to field
java.nio.Bits.MAX_MEMORY
WARNING: Please consider reporting this to the maintainers of
me.clintropolis.sandbox.mem.Main
WARNING: Use --illegal-access=warn to enable warnings of further illegal
reflective access operations
WARNING: All illegal access operations will be denied in a future release
```
The 2nd way involves directly using `jdk.internal.misc.VM` like we were
doing for `sun.misc.VM` something like:
```
Class<?> vmClass = Class.forName("jdk.internal.misc.VM");
Object maxDirectMemoryObj =
vmClass.getMethod("maxDirectMemory").invoke(null);
if (maxDirectMemoryObj == null || !(maxDirectMemoryObj instanceof
Number)) {
throw new UOE("Cannot determine maxDirectMemory from [%s]",
maxDirectMemoryObj);
} else {
return ((Number) maxDirectMemoryObj).longValue();
}
```
and adding:
```
--add-exports java.base/jdk.internal.misc=ALL-UNNAMED
```
to the `java` command-line when running (or stuff in jvm.config?). If this
is not added, it will explode violently with:
```
Exception in thread "main" java.lang.IllegalAccessException: class
me.clintropolis.sandbox.mem.Main cannot access class jdk.internal.misc.VM (in
module java.base) because module java.base does not export jdk.internal.misc to
unnamed module @2f7c7260
at
java.base/jdk.internal.reflect.Reflection.newIllegalAccessException(Reflection.java:361)
at
java.base/java.lang.reflect.AccessibleObject.checkAccess(AccessibleObject.java:591)
at java.base/java.lang.reflect.Method.invoke(Method.java:558)
at me.clintropolis.sandbox.mem.Main.main(Main.java:17)
```
Other approaches I've seen: [Netty has some crazy bit about trying to pull
it from the java command-line
options](https://github.com/netty/netty/blob/4.1/common/src/main/java/io/netty/util/internal/PlatformDependent.java#L1035)
to parse if the user has set `-XX:MaxDirectMemorySize` and try to use that if
it can't get the information from `sun.misc.VM`, but i'm unsure if that is
reasonable to do here.
Personally, I don't really understand why the java developers don't think it
necessary to provide a friendly way to expose this information, but it is what
it is.
I don't know if we want to pursue either of these approaches, just wanted to
bring it up for discussion. All this said, it looks to me like [the jdk itself
defaults max directMemory to
`Runtime.getRuntime().maxMemory()`](https://github.com/AdoptOpenJDK/openjdk-jdk12u/blob/master/src/java.base/share/classes/jdk/internal/misc/VM.java#L208)
if `-XX:MaxDirectMemorySize` is not set, so it doesn't seem so unreasonable to
size off of that...
Regardless, thanks for working on this stuff @xvrl!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
