Skip to content

NativeBufferAllocator: release memory when buffer object is GCed#67

Merged
pavly-gerges merged 60 commits into
Electrostat-Lab:masterfrom
Ali-RS:fix-memory-leak
Apr 6, 2023
Merged

NativeBufferAllocator: release memory when buffer object is GCed#67
pavly-gerges merged 60 commits into
Electrostat-Lab:masterfrom
Ali-RS:fix-memory-leak

Conversation

@Ali-RS
Copy link
Copy Markdown
Contributor

@Ali-RS Ali-RS commented Mar 27, 2023 •
edited
Loading

Fix #66

There are 2 TODOs remaining to be fixed.

@Ali-RS Ali-RS changed the title Fix memory leak NativeBufferAllocator: release memory when buffer object is GCed Mar 27, 2023
@pavly-gerges
Copy link
Copy Markdown
Member

pavly-gerges commented Mar 27, 2023

Hello @Ali-RS, thanks for the PR.
If you would like to continue to fix this issue, please kindly follow the code design illustrated on issue #66, the code should be similar to the one on the issue (ofc with your own optimizations), and we need an example at jme3-alloc-examples to test the code.

@pavly-gerges pavly-gerges added core Concerning API core code incompatibility An incompatibility with the original target. incompatibility-patch Patch to fix the incompatibilities and removed incompatibility An incompatibility with the original target. labels Mar 27, 2023
@pavly-gerges pavly-gerges added this to the v1.0.0-gamma milestone Mar 27, 2023
@Ali-RS
Copy link
Copy Markdown
Contributor Author

Ali-RS commented Mar 27, 2023 •
edited
Loading

please kindly follow the code design illustrated on issue #66, the code should be similar to the one on the issue

Hi @Scrappers-glitch, this PR is designed based on the JME LWJGL 3 buffer allocator, I am not sure I can follow the approach illustrated on issue #66 as I do not understand it fully. Please feel free to close this PR if you prefer a different implementation. Thanks

@pavly-gerges
Copy link
Copy Markdown
Member

pavly-gerges commented Mar 27, 2023

please kindly follow the code design illustrated on issue #66, the code should be similar to the one on the issue

Hi @Scrappers-glitch, this PR is designed based on the JME LWJGL 3 buffer allocator, I am not sure I can follow the approach illustrated on issue #66 as I do not understand it fully. Please feel free to close this PR if you prefer a different implementation. Thanks

No problem, I will try to continue on this PR and add the intended design.

@pavly-gerges pavly-gerges self-assigned this Mar 27, 2023
pavly-gerges and others added 16 commits March 27, 2023 18:48
@pavly-gerges
NativeBufferUtils: reverted code
48a31a1
@pavly-gerges
GarbageCollectibleBuffers: basic API
fcbdc14
@pavly-gerges
GarbageCollectibleBuffers: added jme interface
7ce8ea0
@pavly-gerges
NativeBufferUtils: added NativeBufferUtils#destroy(long)
03eb6df
@pavly-gerges
jmealloc-natives: added native implementation for `NativeBufferUtils#…
1a6201e 
…destroy(long)`
@pavly-gerges
jme-alloc-natives: fixed too few args error for destroy0(void*)
efe2d00
@pavly-gerges
com_jme3_alloc_util_NativeBufferUtils.c: fix `getMemoryAddress(JNIEnv…
3ce4d20 
…*, jobject*)` to return mem-address
@pavly-gerges
GarbageCollectibleBuffers: keep track of created buffer addresses to …
6e3c309 
…avoid duplicated actions
@pavly-gerges
API: refactored NativeBufferUtils#getMemoryAddress(Buffer) to use a…
d2fe2c2 
…bstract Buffer
@pavly-gerges
API: Code-Optimizations
8265ed9
@pavly-gerges
GarbageCollectibles-API: added loggers
4540458
@pavly-gerges
GarbageCollectibles: further optimizations by hiding the Collectibles…
0ecb303 
… MemoryScavenger
@pavly-gerges
GarbageCollectibles: made MemoryScavenger package-private
f4ee922
@pavly-gerges
GarbageCollectibles-API: refactored to OOP
1276740
@pavly-gerges
MemoryScavenger#LOGGER: removed static modifier
55b317e
@pavly-gerges
Merge branch 'fix-memory-leak' of https://github.com/Ali-RS/jme-alloc …
370983a 
…into fix-memory-leak
@pavly-gerges pavly-gerges linked an issue Mar 30, 2023 that may be closed by this pull request
[Deployment] Deploy a concomitant debug binary with each release #65
Closed
@pavly-gerges pavly-gerges temporarily deployed to maven-central March 30, 2023 15:30 — with GitHub Actions Inactive
@pavly-gerges pavly-gerges temporarily deployed to maven-central March 30, 2023 15:31 — with GitHub Actions Inactive
Ali-RS
Ali-RS commented Mar 30, 2023
View reviewed changes
Comment thread jme3-alloc/src/main/java/com/jme3/alloc/gc/GarbageCollectibleBuffers.java Outdated
Ali-RS
Ali-RS commented Mar 30, 2023
View reviewed changes
Comment thread jme3-alloc/src/main/java/com/jme3/alloc/gc/GarbageCollectibleBuffers.java Outdated
Ali-RS
Ali-RS commented Mar 30, 2023
View reviewed changes
Comment thread jme3-alloc/src/main/java/com/jme3/alloc/gc/MemoryScavenger.java Outdated
Ali-RS
Ali-RS commented Mar 31, 2023
View reviewed changes
Comment thread jme3-alloc/src/main/java/com/jme3/alloc/NativeBufferAllocator.java Outdated
Ali-RS
Ali-RS commented Mar 31, 2023
View reviewed changes
Comment thread jme3-alloc/src/main/java/com/jme3/alloc/gc/GarbageCollectibleBuffers.java Outdated
@pavly-gerges pavly-gerges had a problem deploying to release-documentation March 31, 2023 09:21 — with GitHub Actions Failure
@pavly-gerges pavly-gerges temporarily deployed to maven-central March 31, 2023 09:25 — with GitHub Actions Inactive
@pavly-gerges pavly-gerges temporarily deployed to maven-central March 31, 2023 09:28 — with GitHub Actions Inactive
Ali-RS
Ali-RS commented Mar 31, 2023
View reviewed changes
Comment thread jme3-alloc/src/main/java/com/jme3/alloc/gc/GarbageCollectibleBuffer.java
Ali-RS
Ali-RS commented Mar 31, 2023
View reviewed changes
Comment thread jme3-alloc/src/main/java/com/jme3/alloc/gc/GarbageCollectibleBuffers.java Outdated
@pavly-gerges
Copy link
Copy Markdown
Member

pavly-gerges commented Apr 1, 2023

@Ali-RS I would like to release the gamma version and the final stable version during this week, so please, could you help me by adding the examples you suggested on forums at the jme3-alloc-examples module, thank you.

@Ali-RS
Copy link
Copy Markdown
Contributor Author

Ali-RS commented Apr 2, 2023

I would like to release the gamma version and the final stable version during this week

I think we should not rush, there are still issues that need to be fixed.

See: https://hub.jmonkeyengine.org/t/jme-alloc-project/46356/180?u=ali_rs

@pavly-gerges
Copy link
Copy Markdown
Member

pavly-gerges commented Apr 6, 2023

@Ali-RS Thanks for your contributions, this PR will be merged for now, if you want to inquire about the API status don't hesitate to ping me on the forums or here on a new issue, Have a great day!

@pavly-gerges pavly-gerges merged commit b098af9 into Electrostat-Lab:master Apr 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@pavly-gerges pavly-gerges pavly-gerges left review comments

Assignees

@Ali-RS Ali-RS

@pavly-gerges pavly-gerges

Labels

core Concerning API core code incompatibility-patch Patch to fix the incompatibilities

Projects

None yet

Milestone

v1.0.0-gamma

Development

Successfully merging this pull request may close these issues.

[GC] Attach DirectBuffers to GC [Deployment] Deploy a concomitant debug binary with each release [CI/CD] A debug-test job

2 participants

@Ali-RS @pavly-gerges