I believe this is a call to halt the development of this, well, it is always great to learn new stuff, so I am going to merge the current PR and continue learning about memory management as I go, if, for ever reason, someone would like to use the API, just ping me here. Thank you!
I’m probably missing something (because this thread is soooo long and complicated…)
but my understanding is that leaving AS-IS the ReflectionAllocator issue is not a solution.
We should not leave such illegal reflective access operation in jme-core.
My question is : if this allocator is not really useful, why keep using it by default ? Why not using the PrimitiveAllocator instead ?
Just to add some information, the reason LWJGL does not support android has to do with the performance of the memory allocation/deallocation. The operations are much slower than on HotSpot based JVMs. As such Spasi does not create android builds. There is an android branch (iirc it is from 2017) that is a full working version of LWJGL for Android.
The ReflectionAllocator was originally implemented to provide manual memory management on low-memory devices (e.g: Android and Low-memory desktops) until it starts to fail on the latest JDKs and Android (black-flagged reflection access); because sun.* shouldn’t be used on regular java applications, so probably using the Unsafe is not the solution either.
You will never exhibit the Reflection Access error until you make an explicit call to the BufferAllocator#destroyDirectBuffer(Buffer) which is why this is not a priority project in the first place and almost all jMonkeyEngine developers don’t care about this unless you want to implement Instanced Nodes or something that will need direct memory management.
The replacement API for the ReflectionAllocator is generally ready to use, but it needs to be tested on a real application, well maybe something like the Instanced Nodes, but I am not in charge here of integrating it into the engine.
1 Like
Lwjgl-3 doesn’t use the official way to allocate direct memory in Java and they say the official native allocators are not effective, well, yes and no and I don’t think it’s the right path to use Unsafe as Oracle has warned about the use of sun packages before, so that why they may not support android, while Android platform can effectively manage direct memory written in C/C++, and it can be done on java, too.
You will never exhibit the Reflection Access error until you make an explicit call to the BufferAllocator#destroyDirectBuffer(Buffer) which is why this is not a priority project in the first place and almost all jMonkeyEngine developers don’t care about this unless you want to implement Instanced Nodes or something that will need direct memory management.
Yes, I understand applications will just run fine, but we still get a JVM warning (at least with openJDK 11+) on start of any jme desktop application, due to the static initialization of the ReflectionAllocator (which is the default on desktop) :
WARNING: An illegal reflective access operation has occurred
WARNING: Illegal reflective access by com.jme3.util.ReflectionAllocator (file:/home/cedric/.gradle/caches/modules-2/files-2.1/org.jmonkeyengine/jme3-core/3.6.0-stable/4ec2e1cd3cc60d0f1facc78c273dbb4ad13ceca4/jme3-core-3.6.0-stable.jar) to method sun.nio.ch.DirectBuffer.cleaner()
WARNING: Please consider reporting this to the maintainers of com.jme3.util.ReflectionAllocator
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 problem does not arise on Android since we already use the new AndroidNativeBufferAllocator, which supports direct memory management and has no issue.
So my point is : why do we still use the ReflectionAllocator on desktop as the default since it leaves such JVM warning for everybody ?
If an application really needs direct memory management, it can still change the allocator.
I guess the issue is the backward compatibility. But even regarding that, I’m not sure what would be the problem of using the PrimitiveAllocator as the default buffer allocator (on desktop). A call to PrimitiveAllocator#destroyDirectBuffer will be a no-op and the GC will just take care later of releasing the memory. Yes, the behavior is not exactly the same, but hardly noticeable and perfectly fine for 99.999% applications. We could log a warning when an app calls PrimitiveAllocator#destroyDirectBuffer to alert that direct memory management is no longer supported by default and that they should switch to the ReflectionAllocator.
Thus, shouldn’t we either change the default allocator or properly finish the ReflectionAllocator, but not leave the issue AS-IS ?
I’ve seen the JVM warning with Java 11 but not with Java 17+, so it doesn’t seem worth worrying about.
I believe the Weak References can fix the problem of the escape analysis:
I guess we could use this function to manipulate Java Objects from the native code to the Java code, instead of returning a jobject directly:
1 Like
I released another pre-release using Weak Global References instead of Global References in the return statements of the JNI functions.
I tested the changes for some hours (2 hrs) using visualvm, and the performance was comparable to the LwjglBufferAllocator, I will do further testing and try to post some snapshots.
Here we go, if anyone would like to test:
Hello, I ran the example TestReleaseDirectMemory for an hour and captured some memory snapshots, here we go, you can examine them:
https://github.com/Software-Hardware-Codesign/jme-alloc/tree/control-of-references/jme3-alloc-examples
@ 8:12:
"Name","Live Bytes","Live Objects"
"byte[]","1,897,760 B (32.6%)","29,294 (23.6%)"
"java.lang.String","678,576 B (11.7%)","28,274 (22.8%)"
"java.lang.Class","480,816 B (8.3%)","3,818 (3.1%)"
"java.lang.Object[]","271,256 B (4.7%)","4,002 (3.2%)"
"int[]","262,888 B (4.5%)","1,250 (1%)"
"java.util.HashMap$Node","226,144 B (3.9%)","7,067 (5.7%)"
"java.util.concurrent.ConcurrentHashMap$Node","179,104 B (3.1%)","5,597 (4.5%)"
"java.util.HashMap$Node[]","114,144 B (2%)","957 (0.8%)"
"java.lang.reflect.Method","68,376 B (1.2%)","777 (0.6%)"
"char[]","57,352 B (1%)","243 (0.2%)"
...
@ 7:46:
"Name","Live Bytes","Live Objects"
"byte[]","1,957,792 B (30.2%)","30,731 (22.3%)"
"java.lang.String","708,816 B (10.9%)","29,534 (21.4%)"
"java.lang.Class","487,648 B (7.5%)","3,876 (2.8%)"
"int[]","326,496 B (5%)","1,256 (0.9%)"
"java.lang.Object[]","280,800 B (4.3%)","4,331 (3.1%)"
"java.util.HashMap$Node","237,184 B (3.7%)","7,412 (5.4%)"
"java.util.concurrent.ConcurrentHashMap$Node","182,784 B (2.8%)","5,712 (4.1%)"
"java.lang.reflect.Method","182,600 B (2.8%)","2,075 (1.5%)"
"java.util.HashMap$Node[]","145,784 B (2.2%)","1,232 (0.9%)"
"java.util.LinkedHashMap$Entry","97,200 B (1.5%)","2,430 (1.8%)"
"java.lang.Class[]","66,584 B (1%)","2,936 (2.1%)"
"java.lang.String[]","60,064 B (0.9%)","1,708 (1.2%)"
"char[]","57,488 B (0.9%)","248 (0.2%)"