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

Lag caused by adding mesh to MeshInstance3D (possibly due to the use of threads) in Godot 4.4 dev4,5,6,7 and beta1 #101844

Closed
ogyrec-o opened this issue Jan 20, 2025 · 15 comments · Fixed by #102125

Comments

@ogyrec-o
Copy link

ogyrec-o commented Jan 20, 2025

Tested versions

Reproducible in: Godot 4.4 beta1, dev4,5,6,7, Godot 4.4 dev1
Not reproducible in: Godot 4.3 stable

System information

Godot v4.4.beta1 - Windows 10 (build 19045) - Multi-window, 2 monitors - Vulkan (Forward+) - dedicated NVIDIA GeForce RTX 3070 (NVIDIA; 32.0.15.6636) - AMD Ryzen 7 5800X 8-Core Processor (16 threads)

Issue description

In Godot 4.4 dev1 dev4,5,6,7 and 4.4 beta1, a noticeable lag occurs when assigning a new not empty mesh to a MeshInstance3D. This happens specifically on the line:

# sb.gd
func changeMesh() -> void:
	# The lag issue is caused by this line in Godot 4.4 beta1 and dev4,5,6,7.
	# The issue persists from Godot 4.4 dev4,5,6,7 up to and including Godot 4.4 beta1.
	# If the same project is run in Godot 4.3 release, there are no issues.
	# Commenting out this line in Godot 4.4 removes the lag, suggesting the issue might be related to CSG/Mesh handling.
	# Replacing the line with add_child(CSGBox3D.new()) also causes lag in 4.4.
	# However, using add_child(Node3D.new()) works without any lag in 4.4.
	mi.mesh = BoxMesh.new()
	pass

The issue does not appear in Godot 4.3, where the same project works without any performance drops. When running the project in Godot 4.4 with this line active, there is significant lag, which can be resolved by commenting out the line.

Additionally, replacing mi.mesh = BoxMesh.new() with add_child(CSGBox3D.new()) triggers the same lag. However, using add_child(Node3D.new()) works without any lag, suggesting the problem is related to mesh handling or CSG node processing in Godot 4.4.

In my project, there are parallel tasks (threads) being executed through a task manager, similar to what was used in my main project. I’m unsure whether the presence of these tasks is contributing to this issue. I added the same multithreading setup here, but I haven't been able to determine if the lag is caused by the mesh handling or if the task manager's threads are influencing it.

https://youtu.be/Uw34gAoN7HE

Steps to reproduce

Create a new Godot project and copy the provided scripts from MRP into your project or use MRP.

  • main.gd: Attach this script to a Node3D node.
  • sb.gd: Used in main.gd for creating instances of SB.
  • taskManager.gd: This is a helper script to manage tasks using WorkerThreadPool and should be Global.
  • Open the project in the Godot editor and run the scene.

Observe the performance issue:

The project creates 255 instances of the SB node.
Each SB runs a heavy computation (simulated in the somethingProcess() method).
After computation completes, the changeMesh() method is called to change the mesh of the SB instance.
Check the lag that occurs when the changeMesh() method sets mi.mesh = BoxMesh.new(). This will introduce a noticeable lag in Godot 4.4 dev1 dev4,5,6,7 and 4.4 beta1. The lag does not occur in Godot 4.3 stable.

Test different changes:

  • Comment out the line mi.mesh = BoxMesh.new(). This removes the lag.
  • Replace the line with add_child(CSGBox3D.new()). This also causes the lag to appear in Godot 4.4 dev1 dev4,5,6,7 and 4.4 beta1.
  • Replace it with add_child(Node3D.new()). The lag does not occur when using a simple Node3D instead of a meshes.

Minimal reproduction project (MRP)

mrp-meshIssue4.4.zip

@AThousandShips
Copy link
Member

Interacting with the scene tree isn't thread safe in general, and where it is thread safety mechanisms are in place to handle it safely, these can introduce stutter

But this is a bit hard to judge, what happens when not using threading?

@ogyrec-o
Copy link
Author

Interacting with the scene tree isn't thread safe in general, and where it is thread safety mechanisms are in place to handle it safely, these can introduce stutter

But this is a bit hard to judge, what happens when not using threading?

changeMesh() method is always called in the main thread. worker threads only used for simulation of computation in somethingProcess(). when thread finishes its task, it signals the main thread to call changeMesh()

i havent yet tested issue without using threads, as the MRP was designed to copy the behavior of my main project with this issue, which uses same threading logic.

i plan to investigate further and will test how the project behaves without threads to determine whether they the issue. if i find anything relevant, i'll let know.

if theres anything else youd like me to clarify or test

@AThousandShips
Copy link
Member

AThousandShips commented Jan 21, 2025

Where does it transfer to the main thread, missed this when looking

Edit: Yes I can see, this might be just natural delays when updating and creating things

@ogyrec-o
Copy link
Author

Where does it transfer to the main thread, missed this when looking

when task.completed.emit()

# taskManager.gd - line 66
func _process(_delta: float) -> void:
	mutex.lock()
	var completed_tasks: Array = []
	for task in tasks:
		if task.is_completed():
			task.completed.emit()
			tasks.erase(task)
	mutex.unlock()
# main.gd - line 20
task.completed.connect(sbCompleted.bind(sb))

@ogyrec-o
Copy link
Author

this might be just natural delays when updating and creating things

even when i introduce significant delays between mesh changes, there is always noticeable lag when changing 1st mesh (even for something as simple as a boxmesh). interestingly, the lag only disappears once all worker threads have finished their tasks, even if only a one mesh change has been requested during that time

this behavior is absent in Godot 4.3, where the same logic works without any lag or stuttering

@ogyrec-o ogyrec-o changed the title Lag caused by adding mesh to MeshInstance3D (possibly due to the use of threads) in Godot 4.4 dev1 and 4.4 beta1 Lag caused by adding mesh to MeshInstance3D (possibly due to the use of threads) in Godot 4.4 dev4,5,6,7 and beta1 Jan 22, 2025
@ogyrec-o
Copy link
Author

yo, i apologize for the mistake in my report. after further testing, i found that the issue actually starts from version 4.4 dev4, not dev1. the issue is also present in dev5, dev6, dev7, and beta1. however, versions 4.4 dev3 and earlier work as expected without any lag.

im currently investigating problem further and will provide updates as soon as i have more information

@matheusmdx
Copy link
Contributor

I did a test and noticed in 4.4 dev 4 the lag seems to come from the render depth pre-pass and bisect pointed to pr #90400 as the culprit

With pr #90400 (compiled with scu_build=yes production=yes)

Image


Reverting pr #90400

Image


CC @DarioSamo

@ogyrec-o
Copy link
Author

looks like the lag can be fixed if u tweak the mesh setup a bit. the issue seems to be tied to how the mesh is being initialized. for example, instead of directly setting the mesh try this:

func changeMesh() -> void:
    var bm: BoxMesh = BoxMesh.new()
    bm.material = StandardMaterial3D.new()
    bm.material.no_depth_test = true
    bm.material.transparency = 1
    mi.mesh = bm

this helps cut down the lag, even in versions where the issue is present. but: in 4.4 dev3, u didnt need this extra setup, and mesh worked fine without any lag. so, adding those extra lines in 4.4 dev4 seems like a workaround for a regression or something off with the default material settings or rendering in new version

it would be worth digging into whether these changes were intentional or not, and if they were, it would be good to optimize things so the performance doesnt take a hit like this

@clayjohn clayjohn moved this from Unassessed to Bad in 4.x Release Blockers Jan 26, 2025
@DarioSamo
Copy link
Contributor

DarioSamo commented Jan 28, 2025

This is a bit of a multi-faceted problem where we're once more against a perceived regression due to doing something that shouldn't be done and exposing a different problem.

First off, the use case being presented here falls under the things that you should not do as shown in this article. More specifically, the part triggering the stutter is this one.

Surface: Compiled when a frame is about to be drawn and 3D objects were instanced on the scene tree for the first time. This can also include compilation for nodes that aren't even visible on the scene tree. The stutter will occur only on the first frame the node is added to the scene, which won't result in an obvious stutter if it happens right after a loading screen.

Indeed if you wait for the boxes to spawn, you'll see this counter going up the moment they show up. This will result in a stutter in any system the first time the material is seen as Godot has never seen the shader before. This stutter exists in 4.3 as well, especially on a system with a clean shader cache and clean driver pipeline cache, but it's much harder to perceive for another reason.

The reason it usually takes significantly less time in 4.3 is because the thread that both compiles and waits for these pipelines to be available is the render thread. This work however has been delegated to the Worker Thread Pool in 4.4, which means if there's a lot of heavy contention like the case presented in the MRP, it takes a lot longer for the pool to get to compiling the pipeline, resulting in a longer stutter.

I will add a mechanic to mark ubershader compilation as high priority, which sanitizes the issue a bit. But I disagree that perceived regressions like these must be fixed and that work shouldn't be expected of users to avoid stutters. You're not creating the proper setup to perceive these stutters in 4.3 unless you disable the project's pipeline cache and clear your driver cache, and the monitor is a really important indicator here that users with a clean cache will get stutters. You must at least give the engine some hint to be able to figure it out and create the material during loading time, even if it's just by loading an invisible version of the mesh into the tree. That work is no different than what is expected to solve shader stutters.

4.4 having a stutter here despite a driver cache being available is just a side effect of the priority and contention issue I mentioned, and that should hopefully be fixed by a PR addressing that. But, I'll insist the situation presented in the OP will have stutters no matter what version of the engine is used.

@DarioSamo
Copy link
Contributor

DarioSamo commented Jan 28, 2025

This is the PR I mentioned before that adds higher priority for ubershaders: #102125.

Please confirm if it helps to alleviate the issue.

@ogyrec-o
Copy link
Author

This is a bit of a multi-faceted problem where we're once more against a perceived regression due to doing something that shouldn't be done and exposing a different problem.

First off, the use case being presented here falls under the things that you should not do as shown in this article. More specifically, the part triggering the stutter is this one.

Surface: Compiled when a frame is about to be drawn and 3D objects were instanced on the scene tree for the first time. This can also include compilation for nodes that aren't even visible on the scene tree. The stutter will occur only on the first frame the node is added to the scene, which won't result in an obvious stutter if it happens right after a loading screen.

Indeed if you wait for the boxes to spawn, you'll see this counter going up the moment they show up. This will result in a stutter in any system the first time the material is seen as Godot has never seen the shader before. This stutter exists in 4.3 as well, especially on a system with a clean shader cache and clean driver pipeline cache, but it's much harder to perceive for another reason.

The reason it usually takes significantly less time in 4.3 is because the thread that both compiles and waits for these pipelines to be available is the render thread. This work however has been delegated to the Worker Thread Pool in 4.4, which means if there's a lot of heavy contention like the case presented in the MRP, it takes a lot longer for the pool to get to compiling the pipeline, resulting in a longer stutter.

I will add a mechanic to mark ubershader compilation as high priority, which sanitizes the issue a bit. But I disagree that perceived regressions like these must be fixed and that work shouldn't be expected of users to avoid stutters. You're not creating the proper setup to perceive these stutters in 4.3 unless you disable the project's pipeline cache and clear your driver cache, and the monitor is a really important indicator here that users with a clean cache will get stutters. You must at least give the engine some hint to be able to figure it out and create the material during loading time, even if it's just by loading an invisible version of the mesh into the tree. That work is no different than what is expected to solve shader stutters.

4.4 having a stutter here despite a driver cache being available is just a side effect of the priority and contention issue I mentioned, and that should hopefully be fixed by a PR addressing that. But, I'll insist the situation presented in the OP will have stutters no matter what version of the engine is used.

yoo, thank u for detailed response

yea, preloading helps. when i added this to main.gd:

var testmi: MeshInstance3D
var testbm: BoxMesh
func _ready() -> void:
	testmi = MeshInstance3D.new()
	add_child(testmi)
	testbm = BoxMesh.new()
	testmi.mesh = testbm

And modified changeMesh() in sb.gd to:

mi.mesh = get_parent().get_parent().testbm

the lag disappeared in 4.4 beta1

however, im still unsure how i can implement such a preloading approach in my main project. cuz instead of using boxmesh, i create custom arraymesh instances in these threads, and it seems it will be more challenging to preload these in a way that avoids stutters

ur explanation about the pipeline priority and shader stutters was very helpful. ill look into ways to preload the meshes or materials during the loading phase, as u suggested. thanks again for ur time and linked PR, it seems like a great step toward improving this issue

@DarioSamo
Copy link
Contributor

DarioSamo commented Jan 28, 2025

however, im still unsure how i can implement such a preloading approach in my main project. cuz instead of using boxmesh, i create custom arraymesh instances in these threads, and it seems it will be more challenging to preload these in a way that avoids stutters

Pipelines are preloaded and cached as long as the material is instanced in the scene with some surface that is compatible. You could probably make a very simple ArrayMesh that loads this material and instance it on the scene tree (even if invisible) ahead of time and it should work. You can check the behavior of what happens by following the monitors as suggested on the article I linked. If the counter in "surface" doesn't go up when the script is triggered, then you're in the clear.

@clayjohn
Copy link
Member

For clarity, there are two things happening here:

  1. There is a shader stutter due to loading the mesh + material at run time
  2. The stutter got waaaaaaay worse in 4.4 due to the introduction of Ubershaders.

Number 2 is fixed by #102125.

Number 1 can be fixed by having a mesh with that shader instanced in the scene tree at load time as described in the docs and Dario above.

@loerting
Copy link

loerting commented Feb 11, 2025

This issue exists for me in 4.3 stable and 4.4-beta3, but in a slightly different fashion (performance drop / leak)

When dynamically adding MeshInstances3D with a mesh attached to the scene, the process time in the profiler slowly starts to linearly grow. After about 5 minutes my game only has about 5 fps. When removing the mesh from the MeshInstances3D, the game regains the full fps count and the process time drops. The visual profiler shows that "Depth Prepass" and "Render Opaque Pass" both grow. No workaround found yet.

Image

@akien-mga
Copy link
Member

@loerting That's a separate issue, this one was a regression in 4.4 only and has been fixed.

Please open a new bug report for your issue with a minimal reproduction project.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment