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

Surface tool is exponentially slower when threaded #56524

Open
DataPlusProgram opened this issue Jan 5, 2022 · 17 comments
Open

Surface tool is exponentially slower when threaded #56524

DataPlusProgram opened this issue Jan 5, 2022 · 17 comments

Comments

@DataPlusProgram
Copy link

Godot version

3.4.2.stable

System information

Windows 10, GLES3, Radeon 5700xt, R7 5800

Issue description

Fast performance with no threading:
noThread

Slow performance with threading:
withThread

Steps to reproduce

I am aware of #51311 but the difference here is that SurfaceTool.append_from() is not being invoked.

Here is how the code is laid out

var surf = SurfaceTool.new()
var mesh = Mesh.new()

surf.begin(Mesh.PRIMITIVE_TRIANGLES)

	surf.set_material(mat)
		
	
	surf.add_normal(normal)
	surf.add_uv(Vector2(startUVx,startUVy))
	surf.add_vertex(TL)
	
	surf.add_normal(normal)
	surf.add_uv((Vector2(endUVx,startUVy)))
	surf.add_vertex(TR)
	
	surf.add_normal(normal)
	surf.add_uv(Vector2(endUVx,endUVy))
	surf.add_vertex(BR)
	
	
	surf.add_normal(normal)
	surf.add_uv(Vector2(startUVx,startUVy))
	surf.add_vertex(TL)
	
	surf.add_normal(normal)
	surf.add_uv(Vector2(endUVx,endUVy))
	surf.add_vertex(BR)
	
	surf.add_normal(normal)
	surf.add_uv(Vector2(startUVx,endUVy))
	surf.add_vertex(BL)
	
	
	surf.commit(mesh)

Minimal reproduction project

No response

@Zylann
Copy link
Contributor

Zylann commented Jan 5, 2022

Is the slowdown around commit(), by any chance?
Did you try without colliders? (physics isnt thread-safe yet AFAIK)

@fire fire changed the title Surface tool is exponetially slower when threaded Surface tool is exponentially slower when threaded Jan 5, 2022
@Zireael07
Copy link
Contributor

OpenGL shaders don't play nice with threads...

@Calinou
Copy link
Member

Calinou commented Jan 5, 2022

OpenGL shaders don't play nice with threads...

SurfaceTool runs on the CPU, but the mesh needs to be uploaded to the GPU so it can be rendered. There is no shader compilation involved here.

@DataPlusProgram
Copy link
Author

DataPlusProgram commented Jan 5, 2022

I went and checked the time taken for each function individually and in a funny twist of fate it was everything but the surface tool that was causing the slowdown.

The 2 functions causing the slowdown are create_trimesh_collision and lightmap_unwrap
Here's the numbers:

no threading:

create_trimesh_collision called 404 times, on average taking 0.037129ms
lightmap_unwrap called 404 times, on average taking 3.539604ms

threading:

create_trimesh_collision called 404 times, on average taking 99.54703ms
lightmap_unwrap called 404 times, on average taking 63.054455ms

Could someone advise me if these have been documented elsewhere or if I will open an issue for each individually

@Zylann
Copy link
Contributor

Zylann commented Jan 5, 2022

The 2 functions causing the slowdown are create_trimesh_collision

physics isnt thread-safe yet AFAIK

@Calinou
Copy link
Member

Calinou commented Jan 5, 2022

@DataPlusProgram Can you use threads for mesh generation, but go back to a single thread (that waits for the multiple mesh generation threads) to generate the trimesh collision and lightmap UV2?

@DataPlusProgram
Copy link
Author

DataPlusProgram commented Jan 5, 2022

Originally I made it threaded because the editor's importing functionality isn't exposed to GDScript so I have to create a thread and wait for the editor to import each file (generation of .stex, .md5, etc..) on its own time.

Although it's not really a trivial fix I think I can rewrite the plugin in such way that it does a threaded pre-pass importing of resources and after that it will be free to generate meshes in a non-threaded environment.

@fire
Copy link
Member

fire commented Jan 5, 2022

I've noticed that sometimes the threaded mode is [not] really threaded in my tests. I got blocked so I moved on.

@Scony
Copy link
Contributor

Scony commented Jan 25, 2022

Can someone spare a minimal reproduction project? I'd like to take a look as I suspect this one is a synchronization problem just like #57148

@clayjohn
Copy link
Member

You can probably use the MRP here #51311 (comment)

@Scony
Copy link
Contributor

Scony commented Jan 29, 2022

@DataPlusProgram you can see my comment regarding slowdown coming from synchronization on VisualServer level: #51311 (comment)

For your case, creating mesh var mesh = Mesh.new() (and adding it to the scene tree), handling that mesh, and altering mesh surf.commit(mesh) takes a lot of time when done on thread.

IMO, you should limit implementation running on the thread to creating SurfaceTool instances (with SurfaceTool.new()), filling them (using surf.add_ functions), and storing SurfaceTool instances in the array.

Once this is done, you should do the remaining stuff in the main thread (without extra thread). So you'd just have to iterate over that stored SurfaceTool instances in the array and for each of those create a new Mesh instance and commit the surface tool's calculated surface to it.

@pseidemann
Copy link

pseidemann commented Dec 1, 2022

hi all,

do I understand correctly that the issue is that only commit() is slow in a separate thread?

I ran into the same issue and also found out that MeshDataTool's surface read/write methods are also extremely slow in a separate Thread.

is there any good solution to this problem today in 3.5?

edit: shouldn't this issue get the bug label instead of discussion?

@Calinou
Copy link
Member

Calinou commented Dec 1, 2022

edit: shouldn't this issue get the bug label instead of discussion?

It may be an engine limitation that can't be lifted, and can only be documented, especially in 3.x. OpenGL has a lot of limitations around threading, as it's not thread-safe by design.

@pseidemann
Copy link

thanks for the feedback @Calinou.

does this mean there might be improvements regarding this issue in godot 4?

@Calinou
Copy link
Member

Calinou commented Dec 8, 2022

does this mean there might be improvements regarding this issue in godot 4?

I'd suggest trying the same code in 4.0 and see if it runs any better.

@Chaosus
Copy link
Member

Chaosus commented Dec 8, 2022

I hope #69723 will improve the performance of that class

@pseidemann
Copy link

amazing, so we might see improvement in 3.6 already. thanks!

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

No branches or pull requests

9 participants