When I reworked the MemoryManager for Godot 4.0 and started to use Godot's instance binding system, I solved a number of memory leak we had in the Godot 3 version of the module and made the system simpler.
But there is one area I haven't touched yet and doesn't make sense in the current system: The MemoryManager's thead frequency.
Here the current implementation:
/** Minimum time before 2 iterations of the thread.*/
private const val MIN_DELAY = 0L
/** Maximum time before 2 iterations of the thread.*/
private const val MAX_DELAY = 2000L
/** The delay between iterations can be increased or decreased by this value.*/
private const val INC_DELAY = 100L
private fun run() {
while (gcState == GCState.STARTED) {
if (forceJvmGarbageCollector) {
forceJvmGc()
}
val isActive = bindNewObjects() || removeObjectsAndDecrementCounter() || removeNativeCoreTypes()
if (isActive) {
current_delay -= INC_DELAY
current_delay = current_delay.coerceAtLeast(MIN_DELAY)
} else {
current_delay += INC_DELAY
current_delay = current_delay.coerceAtMost(MAX_DELAY)
}
if (current_delay > 0L) {
Thread.sleep(current_delay)
}
}
gcState = GCState.CLOSED
}
Also note that each loop can only check a maximal amount of objects (currently 256)
It was a quick and easily solution to allow the manager to be able to adapt to the amount of work it had. But the reason for that model doesn't apply anymore.
When we were not using instance bindings, we actually had to check periodically ALL Kotlin wrappers to see if the instance was still alive, we were not receiving any callbacks from Godot unlike today. If the frequency was too high, we were making way too much JNI calls and checks (and so mutex locks both in Kotlin and C++), if the frequency was too low, it was taking too long for an object to be garbage collected.
So the more instances we had in memory, the more we had to check, the higher the thread was clocked. It didn't really need to be more complex than this because it would naturally find a stable point, the load only changing with the total amount of objects allocated.
But in Godot 4, our memory manager is entirely based on events. Those events being the creation and destruction of objects. So we need something way more reactive. Most allocations will happen when the game is starting or when specific events happen (loading a level) and the rest of the time it's most likely going to be a close to stable amount of allocation/deallocation each frame. Let's take an example to explain why this system doesn't work:
The game is already started and memory wise in a stable state (so 0 allocation), the memory manager runs at the lowest frequency (currently once every 2s).
We change the current level of the game, so we destroy and create thousands of new object (let's say 2048 of them). The MemoryManager starts to work.
T = 0s, Current Delay: 2s, Next Delay: 1.9s, 256/2048 processed.
T = 2s, Current Delay: 1.9s, Next Delay: 1.8s, 512/2048 processed.
Many loops later
T=13,2s, Current Delay: 1.3s, Next Delay: 1.2s, 2048/2048 processed.
Overall, it took more than 13s to bind every instance and free memory which is not acceptable.
There is no need to use any variable delay anymore, we can just set for a short constant delay by default (like 100ms) and not pause the thread until all the work is done.
Another opportunity to take in that rework would be to batch the instances that need to be bound/unbound instead of making individual JNI calls for them. It would help performances.
When I reworked the MemoryManager for Godot 4.0 and started to use Godot's instance binding system, I solved a number of memory leak we had in the Godot 3 version of the module and made the system simpler. But there is one area I haven't touched yet and doesn't make sense in the current system: The MemoryManager's thead frequency. Here the current implementation:
Also note that each loop can only check a maximal amount of objects (currently 256)
It was a quick and easily solution to allow the manager to be able to adapt to the amount of work it had. But the reason for that model doesn't apply anymore. When we were not using instance bindings, we actually had to check periodically ALL Kotlin wrappers to see if the instance was still alive, we were not receiving any callbacks from Godot unlike today. If the frequency was too high, we were making way too much JNI calls and checks (and so mutex locks both in Kotlin and C++), if the frequency was too low, it was taking too long for an object to be garbage collected. So the more instances we had in memory, the more we had to check, the higher the thread was clocked. It didn't really need to be more complex than this because it would naturally find a stable point, the load only changing with the total amount of objects allocated.
But in Godot 4, our memory manager is entirely based on events. Those events being the creation and destruction of objects. So we need something way more reactive. Most allocations will happen when the game is starting or when specific events happen (loading a level) and the rest of the time it's most likely going to be a close to stable amount of allocation/deallocation each frame. Let's take an example to explain why this system doesn't work:
Overall, it took more than 13s to bind every instance and free memory which is not acceptable. There is no need to use any variable delay anymore, we can just set for a short constant delay by default (like 100ms) and not pause the thread until all the work is done.
Another opportunity to take in that rework would be to batch the instances that need to be bound/unbound instead of making individual JNI calls for them. It would help performances.