Skip to content

Memory allocators overhaul#2835

Merged
riccardobl merged 14 commits into
jMonkeyEngine:masterfrom
riccardobl:allocators
May 29, 2026
Merged

Memory allocators overhaul#2835
riccardobl merged 14 commits into
jMonkeyEngine:masterfrom
riccardobl:allocators

Conversation

@riccardobl
Copy link
Copy Markdown
Member

@riccardobl riccardobl commented May 29, 2026

This PR improves memory allocators in every backend

  • add memory management to android and ios allocators: closes AndroidNativeBufferAllocator memory leak #1990
  • update to saferalloc 0.0.10 with iOS support and 16kb pages for android
  • default to saferalloc based allocators in ios and android when jme3-saferallocator dependency is present
  • rework LWJGL3 allocator to be fully threadsafe
  • deprecate ConcurrentLWJGLBufferAllocator and turn it to an alias of LWJGLBufferAllocator (since it is threadsafe now)
  • deprecate ReflectionAllocator that is not supported in modern java nor guaranteed to work in non hostpot jvms
  • fix some issues with saferalloc memory pressure estimation
  • use saferalloc in screenshot tests

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request updates dependency versions and integrates the SaferBufferAllocator dynamically across Android and iOS platforms. It also refactors AndroidNativeBufferAllocator and LibJGLIOSNativeBufferAllocator to automatically clean up native memory using phantom references and background threads, while simplifying and deprecating the concurrent LWJGL allocator. The review feedback suggests several robustness and performance improvements: throwing an explicit OutOfMemoryError when allocation fails in AndroidNativeBufferAllocator, wrapping the background deallocation loops in try-catch blocks to prevent silent thread termination, and using AtomicIntegerFieldUpdater instead of AtomicBoolean in the Deallocator classes to reduce garbage collection overhead.

@github-actions
Copy link
Copy Markdown

🖼️ Screenshot tests have failed.

The purpose of these tests is to ensure that changes introduced in this PR don't break visual features. They are visual unit tests.

📄 Where to find the report:

⚠️ If you didn't expect to change anything visual:
Fix your changes so the screenshot tests pass.

If you did mean to change things:
Review the replacement images in jme3-screenshot-tests/build/changed-images to make sure they really are improvements and then replace and commit the replacement images at jme3-screenshot-tests/src/test/resources.

If you are creating entirely new tests:
Find the new images in jme3-screenshot-tests/build/changed-images and commit the new images at jme3-screenshot-tests/src/test/resources.

Note; it is very important that the committed reference images are created on the build pipeline, locally created images are not reliable. Similarly tests will fail locally but you can look at the report to check they are "visually similar".

See https://github.com/jMonkeyEngine/jmonkeyengine/blob/master/jme3-screenshot-tests/README.md for more information

Contact @richardTingle (aka richtea) for guidance if required

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR overhauls direct buffer allocation across LWJGL3, Android, and iOS, adding safer allocator selection and PhantomReference-based native memory cleanup for mobile backends.

Changes:

  • Adds GC-backed native buffer deallocation for Android and iOS allocators.
  • Refactors LWJGL3 allocation bookkeeping to be thread-safe and deprecates the concurrent allocator alias.
  • Adds saferalloc support/version updates and wires examples to include jme3-saferallocator.

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
jme3-lwjgl3/src/main/java/com/jme3/util/LWJGLBufferAllocator.java Refactors deallocator lifecycle with atomic free guards and deprecates the concurrent subclass.
jme3-lwjgl3/src/main/java/com/jme3/system/lwjgl/LwjglContext.java Selects saferalloc when available and updates LWJGL allocator defaults.
jme3-ios/src/main/java/com/jme3/util/LibJGLIOSNativeBufferAllocator.java Adds PhantomReference tracking and address-based native freeing.
jme3-ios/src/main/java/com/jme3/system/ios/IGLESContext.java Prefers SaferBufferAllocator when present, otherwise uses the iOS native allocator.
jme3-ios-examples/build.gradle Adds saferallocator dependency to iOS examples.
jme3-core/src/main/java/com/jme3/util/ReflectionAllocator.java Marks reflection allocator as deprecated.
jme3-android/src/main/java/com/jme3/util/AndroidNativeBufferAllocator.java Adds PhantomReference tracking and changes JNI methods to address-based freeing.
jme3-android/src/main/java/com/jme3/app/AndroidHarnessFragment.java Selects saferalloc/native allocator before application creation.
jme3-android/src/main/java/com/jme3/system/android/OGLESContext.java Selects saferalloc when available for Android context initialization.
jme3-android-examples/build.gradle Adds saferallocator dependency to Android examples.
gradle/libs.versions.toml Bumps libjglios/saferalloc and adds iOS saferalloc native artifact.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread jme3-core/src/main/java/com/jme3/util/ReflectionAllocator.java Outdated
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@riccardobl riccardobl marked this pull request as draft May 29, 2026 10:02
@riccardobl
Copy link
Copy Markdown
Member Author

/gemini review again, i did not apply the AtomicIntegerFieldUpdater suggestion because it is a micro-optimization not worth the complexity

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces phantom-reference-based automatic deallocation for direct byte buffers in the Android and iOS native allocators to prevent native memory leaks. It also integrates the jme3-saferallocator module as a preferred allocator on Android and iOS when available, deprecates ReflectionAllocator and ConcurrentLWJGLBufferAllocator, and refactors LWJGLBufferAllocator to use AtomicBoolean for thread-safe deallocation. Feedback suggests using primitive long instead of boxed Long for the native memory address in LWJGLBufferAllocator to avoid unnecessary boxing overhead.

Comment thread jme3-lwjgl3/src/main/java/com/jme3/util/LWJGLBufferAllocator.java Outdated
Comment thread jme3-lwjgl3/src/main/java/com/jme3/util/LWJGLBufferAllocator.java Outdated
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.

riccardobl and others added 2 commits May 29, 2026 12:20
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
@riccardobl riccardobl marked this pull request as ready for review May 29, 2026 10:35
@riccardobl riccardobl changed the title Allocators overhaul Memory allocators overhaul May 29, 2026
@riccardobl riccardobl merged commit 0368453 into jMonkeyEngine:master May 29, 2026
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

AndroidNativeBufferAllocator memory leak

2 participants