godotengine/godot

C# GC not cleaning up InputEvent objects very quickly

Closed this issue · 5 comments

m50 commented

Godot version:
3.2.2.stable.mono

OS/device including version:
Retina 5k 2019 Mac, Radeon Pro 570X 4 G, 40GB memory, 3 GHz 6-Core Intel Core i5

Issue description:
When using _Input/_input in C#, the objects count (as seen in the Debugger > Monitors) climbs and climbs until you stop moving your mouse, at which point it pauses, then all of them are cleaned up. In GDScript, this does not climb this way, and the InputEvent objects are cleaned up immediately.

It appears that the C# garbage collector is holding onto the InputEvent objects for longer than the gdscript garbage collector.

This can cause memory usage to slowly climb, and FPS to slowly drop. While not an issue on my machine, it can be problematic for others.

Steps to reproduce:

Create a C# script that has public override void _Input(InputEvent @event) in it, and run that in a scene.

Minimal reproduction project:

https://github.com/m50/GodotCSharpInputEventGCBug

There is a gdscript scene and a c# scene, both have identical scripts besides language.

m50 commented

I will note: This might be an issue with all Reference objects, it's just VERY obvious with InputEvents.

I think this is by design, that's how garbage collection works. GDScript doesn't exhibit this as it's not GC-ed, but reference counted.
CC @neikeq

GDScript does automatic reference counting.
In C# you can't control when an object is garbage collected. You can manually dispose the input event though.

m50 commented

If this is by design, then excuse me for saying, but the design is bad.

At the very least, the docs need to tell you that you MUST run Dispose on it, or else you will get performance problems, however I would consider this a memory leak, and thus a bug and needs fixing.

I get that C# is supposed to handle the garbage collection, but there should really be something done about this, because it is a memory leak, and a problem.

One of the main reasons to use C# is for the performance benefit, but this can cause the performance to get worse than gdscript.

It's not a memory leak, the memory will be released when the garbage collector finds necessary.

However the fact we have to allocate a new object for each input event can cause issues. I already dislike the fact we're allocating a Reference in C++ for each input event, but it gets worse in C# as it stresses the GC. The latter may be solvable with some form of object pooling, but it would probably still require manual disposal.

I've thought that in the worst case we could add an overload to _Input that takes a struct representation of InputEvent to avoid stressing the GC.

An alternative to manual disposal could be documenting that a reference to the input event passed to _Input must not escape the function call, and as such whoever calls _Input (the engine in this case) takes care of disposing it for you.