Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MemoryManager rework #661

Merged
merged 37 commits into from
Aug 31, 2024
Merged

MemoryManager rework #661

merged 37 commits into from
Aug 31, 2024

Conversation

CedNaru
Copy link
Member

@CedNaru CedNaru commented Aug 17, 2024

Solve #508, #618, #566, #666 and #617

It's honestly hard to explain in details all the changes without turning it into an essay. Most of the changes and explanations were already written in #618.

Here's a recap of the main changes:

  • The MemoryManager is no longer its own thread, instead it's called directly by the C++ code at the end of each frame.
  • Bindings no longer keep JNI references to their JVM wrappers. Objects are directly stored as strong references in the JVM ObjectDB and RefCounted as weak references.
  • The "binding phase" that was made in a delayed manner is now gone. Instead, it's replaced by a "dead object" phase. Each frame, the list of dead objects is sent to the JVM to remove them from the ObjectDB.
  • Refcounted wrappers are no longer permanent, they die as soon as they are no longer used by the JVM, decrementing the counter in the process. The JVM is no longer the one with the final words on RefCounted wrappers' life.
  • The only exception are RefCounted Scripts. The ping pong of weak/strong JNI ref was kept but moved from KoltinBinding to JvmInstance. The ping pong is also less frequent. References are transformed into strong ones immediately, but the switch to weak is delayed to the end of the frame.
  • ForceGC was removed, it's not a reliable method to make sure the GC ran.
  • DisplayLeaks was removed, Godot should be the only one responsible for such reports.
  • Singleton management has been removed because no longer relevant in the new model.
  • Static management has been replaced by a broader "cleanup callback" system. It can be used to close third party threads, for example.

Copy link
Contributor

@chippmann chippmann left a comment

Choose a reason for hiding this comment

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

Very nice work!
Will follow up with proper testing later this week

src/language/gdj_language.cpp Outdated Show resolved Hide resolved
src/script/jvm_instance.cpp Outdated Show resolved Hide resolved
chippmann
chippmann previously approved these changes Aug 30, 2024
Copy link
Contributor

@chippmann chippmann left a comment

Choose a reason for hiding this comment

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

@CedNaru CedNaru requested a review from chippmann August 30, 2024 20:18
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.

3 participants