godot icon indicating copy to clipboard operation
godot copied to clipboard

4.3.rc1 has some memory leak

Open yythlj opened this issue 1 year ago • 13 comments

Tested versions

4.3.rc1 win10

System information

win10

Issue description

memory leaf

Steps to reproduce

I create multi different world3d window and then close them on one frame

wait some minutes and see the memory

if the total window num is more ,the process's memory be more(100 window is 59m, 200 window is 62m) and the monitor see no object more, in 100 window and 200 window, after close the monitor show the same info. image image image

they all show 1568

Minimal reproduction project (MRP)

NA

yythlj avatar Jul 28 '24 15:07 yythlj

and I open a empty project never open the game window it also show 1568 but the memory only 34m image

yythlj avatar Jul 28 '24 15:07 yythlj

if it is my mistake? I get no different from the monitor after close window

yythlj avatar Jul 28 '24 15:07 yythlj

Please provide some more details, you don't explain what you are doing or what you expect, how and when are you freeing the nodes you're using?

AThousandShips avatar Jul 28 '24 15:07 AThousandShips

@yythlj You need to provide a minimal reproduction project if you want us to be able to debug the engine, if a said leak exists.

adamscott avatar Jul 28 '24 17:07 adamscott

okay,I upload now, very simple mini

yythlj avatar Jul 28 '24 17:07 yythlj

I have test this half day on my room

yythlj avatar Jul 28 '24 17:07 yythlj

empty window mode image

100 window mode image

after close 100 window image

@AThousandShips @adamscott

yythlj avatar Jul 28 '24 17:07 yythlj

its a very simple mini proj, only has a simple level scene

yythlj avatar Jul 28 '24 17:07 yythlj

I delete the .godot dir you should reopen it so no error

yythlj avatar Jul 28 '24 18:07 yythlj

@yythlj This isn't an MRP, this is a whole project. The "simple" level scene calls other code that runs. Please create a new project from scratch and try to recreate the memory leak you're having.

adamscott avatar Jul 28 '24 18:07 adamscott

you change the param I picture, and you can see its a simple enough i think... a window, a world, a scene ,a network

---Original--- From: "Adam @.> Date: Mon, Jul 29, 2024 02:56 AM To: @.>; Cc: @.@.>; Subject: Re: [godotengine/godot] 4.3.rc1 has some memory leak (Issue #94878)

@yythlj This isn't an MRP, this is a whole project. The "simple" level scene calls other code that runs. Please create a new project from scratch and try to recreate the memory leak you're having.

— Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you were mentioned.Message ID: @.***>

yythlj avatar Jul 28 '24 19:07 yythlj

you change the param I picture, and you can see its a simple enough i think... a window, a world, a scene ,a network

What I mean is that there's a lot that can happen in that code that could leak. Leaks happen all the time, it's just leaks that happen internally in the engine that are worth debugging and fixing.

And no, it's not simple enough. Try to isolate, remove code, to the point that you can create a new project and replicate the leak easily, without needing a framework for it to happen.

adamscott avatar Jul 28 '24 22:07 adamscott

okay, wait some minutes I delete the code no use some code is no use,I confirm there only a net work on each world I say

no dynamic nodw create any more

---Original--- From: "Adam @.> Date: Mon, Jul 29, 2024 06:21 AM To: @.>; Cc: @.@.>; Subject: Re: [godotengine/godot] 4.3.rc1 has some memory leak (Issue #94878)

you change the param I picture, and you can see its a simple enough i think... a window, a world, a scene ,a network

What I mean is that there's a lot that can happen in that code that could leak. Leaks happen all the time, it's just leaks that happen internally in the engine that are worth debugging and fixing.

And no, it's not simple enough. Try to isolate, remove code, to the point that you can create a new project and replicate the leak easily, without needing a framework for it to happen.

— Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you were mentioned.Message ID: @.***>

yythlj avatar Jul 29 '24 01:07 yythlj

only 9 file.gd is use, it can be a mini proj?

---Original--- From: "Adam @.> Date: Mon, Jul 29, 2024 06:21 AM To: @.>; Cc: @.@.>; Subject: Re: [godotengine/godot] 4.3.rc1 has some memory leak (Issue #94878)

you change the param I picture, and you can see its a simple enough i think... a window, a world, a scene ,a network

What I mean is that there's a lot that can happen in that code that could leak. Leaks happen all the time, it's just leaks that happen internally in the engine that are worth debugging and fixing.

And no, it's not simple enough. Try to isolate, remove code, to the point that you can create a new project and replicate the leak easily, without needing a framework for it to happen.

— Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you were mentioned.Message ID: @.***>

yythlj avatar Jul 29 '24 02:07 yythlj

now this is a very simple 5 gd file without any logic only create scene and free

all code line num less than 500

if this not a mini ,you can test by yourself

---Original--- From: "Adam @.> Date: Mon, Jul 29, 2024 06:21 AM To: @.>; Cc: @.@.>; Subject: Re: [godotengine/godot] 4.3.rc1 has some memory leak (Issue #94878)

you change the param I picture, and you can see its a simple enough i think... a window, a world, a scene ,a network

What I mean is that there's a lot that can happen in that code that could leak. Leaks happen all the time, it's just leaks that happen internally in the engine that are worth debugging and fixing.

And no, it's not simple enough. Try to isolate, remove code, to the point that you can create a new project and replicate the leak easily, without needing a framework for it to happen.

— Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you were mentioned.Message ID: @.***>

yythlj avatar Jul 29 '24 02:07 yythlj

GD4_TPS_Server_mini.zip

@adamscott

yythlj avatar Jul 29 '24 02:07 yythlj

This is still really not minimal. I gave up after 1 minute of importing gltf files which are probably not relevant to the issue (200+ models).

You need to do the work to reduce it further to only what's necessary for the leak to be apparent. In the process, you might find that the bug lies in your code, and not in the engine. If it lies in the engine, then this is indeed a Godot bug for us to look into. But for now this is not clearly established so this issue isn't actionable for contributors.

akien-mga avatar Jul 29 '24 09:07 akien-mga

i alreay has say you should open (wait import finish)and  reopen it because the .godot dir has be deleted

or you team can fix this godot bug

delete .godot dir then next time need to open and reopen. or import file will show error

---Original--- From: "Rémi @.> Date: Mon, Jul 29, 2024 17:57 PM To: @.>; Cc: @.@.>; Subject: Re: [godotengine/godot] 4.3.rc1 has some memory leak (Issue #94878)

This is still really not minimal. I gave up after 1 minute of importing gltf files which are probably not relevant to the issue.

You need to do the work to reduce it further to only what's necessary for the leak to be apparent. In the process, you might find that the bug lies in your code, and not in the engine. If it lies in the engine, then this is indeed a Godot bug for us to look into. But for now this is not clearly established so this issue isn't actionable for contributors.

— Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you were mentioned.Message ID: @.***>

yythlj avatar Jul 29 '24 10:07 yythlj

GD4_TPS_Server_mini.zip image final mini version

yythlj avatar Jul 29 '24 10:07 yythlj

also this final miny maybe can not see the big problem

because maybe relate to the thing that you even no will wait 1 minute img_v3_02d8_ea5a56be-d9b1-4eee-9baf-1305fac35fcg

img_v3_02d8_284b5c66-c0b1-4890-a477-d212386045cg

yythlj avatar Jul 29 '24 10:07 yythlj

Thanks, that's a bit better. There's still too much complexity but after reproducing what I believe you refer to as a leak, and making it more obvious, I simplified it further.

Here's an actual minimal reproduction project that shows the same symptom, i.e. creating and freeing a high number of nodes leaves some residual static memory usage.

static-memory-node.zip

extends Node

const room_max = 3000

var instances = []

var timer = Timer.new()

var test_count = 0
var init = true
const test_max = 3

func _ready():
	timer.wait_time = 3.0
	timer.autostart = true
	timer.timeout.connect(_on_timer_timeout)
	add_child(timer)

func _on_timer_timeout() -> void:
	if init:
		for i in range(room_max):
			var node = Window.new()
			instances.append(node)
			add_child(node)
		init = false
	else:
		for i in instances.size():
			var node = instances.pop_back()
			node.free()
		init = true
		test_count += 1
	if test_count >= test_max:
		timer.stop()
		return

This adds and removes 3000 nodes 3 times in a row, whenever a 3.0s timer ticks, then closes.

The monitors look like this:

image

Static memory goes from 40.93 MiB to 95.63 MiB after instantiating the nodes, down to 45.93 MiB after freeing them. Worth noting that on each "test" (init / free) the total static memory after init seems to increase gradually (95.63 -> 95.75 -> 96.05), but it always goes back to 95.63 MiB (in that process, restarting the editor yields slightly different values).

So there's 5.00 MiB of static memory that has been allocated in the process and doesn't get freed.

Now to call that a leak requires further research, this might well be intended behavior.

Swapping Window.new() for Node.new() produces a smaller difference, but there's still one:

image

It goes from 40.93 MiB to 44.66 MiB and then down to 41.24 MiB (so 0.31 MiB static memory doesn't get freed). Here also there's a slight difference between each peak where it increases gradually.

akien-mga avatar Jul 29 '24 12:07 akien-mga

So one source of static memory use that won't be freed is the object database, it never shrinks, it adds some ~16~ 32 bytes per object, for 3000 nodes thrice over, assuming they don't share the same ObjectID slot, that's just ~30 bytes for 0.31 MiB of memory, so that checks out I'd say (though it looks like the static memory doesn't grow between creation/deletion, so then it'd be some 108 bytes), hard to say for Window but it allocates a lot more other things, like various RIDs

RID owners also do not free up organizational memory when freeing RIDs it seems

This sounds like various similar things

Note also that Window creates new objects itself, as does Viewport which in turn will be allocated up data

Will play around with the MRP when I can and test some different senarios to see how it grows and how it relates to the number of objects and order etc.

Some ideas for pinning sources of issues down:

  • The nature of hierarchy, one potential source of memory overhead would be the child cache vector of Node which shouldn't shrink when removing children unless I miss something in LocalVector, as well as any HashMap in Node, this would be checked by removing the node that holds the children itself
  • The relationship with the maximum number of objects present at any one time, if creating 1000 and then 3000 yields different results from 3000 and then 3000
  • The direct relationship between count and memory

Potential sources of memory allocation:

  • The parent node having HashMaps and LocalVectors for tracking children, these types do not generally shrink when elements are removed
  • Signals connected to various other nodes and objects, they are added as connections to both ends which are also HashMaps, various signals are connected to SceneTree as well as other classes, these would linger after freeing (but only to a maximal size)
  • Allocation of tracking data in ObjectDB
  • Allocation of tracking in various servers, like RIDs

Edit: Note that a lot of containers don't hold exactly the data size either, for example ObjectDB allocates in powers of two, so at least 4096 with 3000 Nodes, and HashMap allocates in specific primes, so in this case at least 3079 with 3000 elements, LocalVector also does powers of two, in this case you'd look at at least 4096 * 32 + 4096 * 8 + 3079 * 12 bytes, that's some 0.196 MiB there

AThousandShips avatar Jul 29 '24 12:07 AThousandShips

Now assuming the above is the case (will try doing some research this week) it is largely by design, but some approaches to reduce the cost would be to divide up the hierarchy and not have too many children in one node, and remove any holding node that contains many nodes at times

For the design: Containers generally are not shrunk, only grown, because of a serious performance issue that can occur, namely that if a container is grown, and then shrunk, across some boundary of standard size (for example a power of two, or a prime number) it reallocates, this can cause thrashing, where it grows by 5 elements, has to be resized, and then shrinks by 4, and is shrunk, and so on, this would hurt performance significantly and is (I'd assume) why this is how our containers are generally designed (at least some that are more performant)

But will see if there aren't some potential hidden things that could be fixed

AThousandShips avatar Jul 29 '24 13:07 AThousandShips

IMO the main takeaways are:

  • This might need to be better documented, I think it's not the first time someone reports a "memory leak" that's just things working as designed.
  • We should maybe see how to surface that share of memory in the profiler, so that users understand what's taking this extra memory (and possibly make it clear e.g. in a tooltip that this memory will never shrink by design).

akien-mga avatar Jul 29 '24 13:07 akien-mga

I agree, after I've taken some further testing when I find the time I can look into working out some ideas for tracking extra memory independently, and word something for documentation

AThousandShips avatar Jul 29 '24 13:07 AThousandShips

cc @AThousandShips Can still replicate on v4.4.1.stable, on Linux (with Akien's MRP):

Image

I've gathered values from 3000 and 30000 objects, and interestingly, the size per object appears to be the same:

using 3000 objects

start: 42.82MB
end:   48.47MB

delta: 5.65MB
size per object: 1.88333KB (5.65MB / 3000)

-----

using 30000 objects (x10)

start: 42.82MB
end:   97.16MB

delta: 54.34MB
size per object: 1.811333KB (54.34MB / 30000)

rsubtil avatar May 06 '25 17:05 rsubtil

In a lot of places in the engine we use selfish allocation strategies that grow, but never release memory so that they can always be prepared for worst-case allocations. So leaving some memory behind is indeed expected. What is important is that the total doesn't grow after doing repeated allocations/frees.

While I think 1.8 kb per node is surprisingly high, I don't think it is enough to be considered a bug. I think Akien's takeaways above still stand https://github.com/godotengine/godot/issues/94878#issuecomment-2255930222

clayjohn avatar May 06 '25 21:05 clayjohn