Deprecating ReflectionAllocator?

To make the long story short, after java 11, the ReflectionAllocator keeps breaking again and again! (At least it does on Linux, I do not know about Windows or Mac)

SEVERE: Buffer cannot be destroyed: java.nio.DirectFloatBufferU

By a glance at this class source code, you can see the code smells! :wink:

with java 11+ we are required to add this VM option to fix it

"--add-opens=java.base/jdk.internal.ref=ALL-UNNAMED"

but it broke up again in java 16+, which now requires also adding:

"--add-exports=java.base/sun.nio.ch=ALL-UNNAMED"

I do not know when it will break again.

But should we expect developers to know about this and add them? I believe not!

Lwjgl3 already has its own native allocator, and thanks to the contribution from @Pavl_G, Android now has its own native allocator in the JME 3.6 too. Leaving out iOS, only lwjgl2 now uses ReflectionAllocator.

With the behind the scene works that we have done on preparing lwjgl2 to run on java 11+ on Linux, the ReflectionAllocator is the only barrier left I believe.

I am proposing to deprecate ReflectionAllocator and make the PrimitiveAllocator the default one, which will let the GC clean the buffers on lwjgl2. I believe this is no more a concern in today’s java, especially with the arrival of ZGC and Shenandoah GC.

If someone has any feedback or concerns about this please let me know here before it is too late!

Edit:

Here is the test case if you want to try it with java 11+

Regards

5 Likes

When you say the “default one”, you mean that JME specifically declares a different one? Not that applications must choose.

Just making sure that I understand.

1 Like

Yes, JME specifies “ReflectionAllocator” by default in BufferAllocatorFactory but applications can still overwrite it by passing a system property.

1 Like

I may have to try setting that system property on my own apps to see if it’s harmful. I do a LOT of allocating. lol.

3 Likes

This is the source code of PrimitiveAllocator that I am proposing to pick by default.

But we must remove that System.err.println(); warning thought, to prevent spamming console!

Are you manually destroying them using BufferUtils.destroyDirectBuffer()?

No. Is that the only case where this comes up? I thought JME was doing its own buffer freeing or something.

I gave up manual deallocation a long time ago. Since Java 1.7 it hasn’t seemed to be an issue at all.

1 Like

Well, I believe yes, if they are wrapped in JME NativeObject like VertexBuffer,…

@Ali_RS, i tracked the lwjgl2 native sources includes and this is the header utilized by jni:

The problem is it creates a new direct buffer using the jni function (NewDirectBuffer) from the java nio api, and currently there is no a native destructor method, neither in lwjgl-2 nor in java NIO API, however there is a GetDirectBufferAddress which returns a pointer that i think can be freed using the GNU standard library (stdlib), so i think we can implement this…

EDIT:
This is the documentation for the GetDirectBufferAddress:

GetDirectBufferAddress
void* GetDirectBufferAddress(JNIEnv* env, jobject buf);

Fetches and returns the starting address of the memory region referenced by the given direct java.nio.Buffer.

This function allows native code to access the same memory region that is accessible to Java code via the buffer object.

LINKAGE:
Index 230 in the JNIEnv interface function table.

PARAMETERS:
env: the JNIEnv interface pointer

buf: a direct java.nio.Buffer object (must not be NULL)

RETURNS:
Returns the starting address of the memory region referenced by the buffer. Returns NULL if the memory region is undefined, if the given object is not a direct java.nio.Buffer, or if JNI access to direct buffers is not supported by this virtual machine.
1 Like

So why are we STILL clinging at lwjgl2, which is practically dead, even when releasing a new version? The body can be hidden, but the rotting smell will keep haunting us…

Yes I know there are N reasons for doing so, unfortunately…

3 Likes

I drafted a PR

It does not deprecate ReflectionAllocator just makes PrimitiveAllocator the default one.

4 Likes