godotengine / godot

Godot Engine – Multi-platform 2D and 3D game engine
https://godotengine.org
MIT License
89.43k stars 20.25k forks source link

Surface tool is exponentially slower when threaded #56524

Open DataPlusProgram opened 2 years ago

DataPlusProgram commented 2 years ago

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 https://github.com/godotengine/godot/issues/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 commented 2 years ago

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

Zireael07 commented 2 years ago

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

Calinou commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

The 2 functions causing the slowdown are create_trimesh_collision

physics isnt thread-safe yet AFAIK

Calinou commented 2 years ago

@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 commented 2 years ago

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 commented 2 years ago

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

Scony commented 2 years ago

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 commented 2 years ago

You can probably use the MRP here https://github.com/godotengine/godot/issues/51311#issuecomment-894250114

Scony commented 2 years ago

@DataPlusProgram you can see my comment regarding slowdown coming from synchronization on VisualServer level: https://github.com/godotengine/godot/issues/51311#issuecomment-1024966544

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

thanks for the feedback @Calinou.

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

Calinou commented 1 year ago

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 commented 1 year ago

I hope https://github.com/godotengine/godot/pull/69723 will improve the performance of that class

pseidemann commented 1 year ago

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