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

Use AHashMap for SurfaceTool #101273

Merged

Conversation

Nazarwadim
Copy link
Contributor

Makes index method 6x faster and generate_normals 3x faster.

Tested in https://github.com/godotengine/godot-demo-projects/tree/master/3d/voxel.

In the _generate_chunk_mesh method of the Chunk class, I used Time.get_ticks_usec to get the elapsed time.

Master:

Total mesh generation time:1990
Generate normals time:132
index time:798
 
Total mesh generation time:2402
Generate normals time:150
index time:980
 
Total mesh generation time:1976
Generate normals time:122
index time:786
 
Total mesh generation time:2403
Generate normals time:158
index time:967
 
Total mesh generation time:2205
Generate normals time:132
index time:875
 
Total mesh generation time:2415
Generate normals time:154
index time:970
 
Total mesh generation time:2590
Generate normals time:165
index time:1039

Current PR:

Total mesh generation time:1556
Generate normals time:69
index time:182
 
Total mesh generation time:1350
Generate normals time:60
index time:157
 
Total mesh generation time:1558
Generate normals time:72
index time:184
 
Total mesh generation time:1087
Generate normals time:51
index time:132
 
Total mesh generation time:1408
Generate normals time:61
index time:161
 
Total mesh generation time:1709
Generate normals time:90
index time:153
 
Total mesh generation time:1348
Generate normals time:59
index time:156

It is worth noting that GDScript has now become the bottleneck, and if we were to do the same generation in C#, the total generation time would decrease many times.

Copy link
Member

@clayjohn clayjohn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall this looks great and the benchmarking results are fantastic! I just left one comment with a question + a suggestion

Copy link
Member

@clayjohn clayjohn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code looks good.

I tested locally using an OBJ version of this mesh (https://sketchfab.com/3d-models/thunderjaw-from-horizon-zero-dawn-34acb03c841247d28aaa80204f1f52b2). SurfaceTool is used to import OBJ files, and it always calls index. So its a good way to test real world performance without the overhead of GDScript.

I saw about a 5% decrease in overall import time for the file, which is pretty significant.

The final vertex count is exactly the same. So this should be totally safe

@clayjohn clayjohn modified the milestones: 4.x, 4.4 Jan 9, 2025
@akien-mga akien-mga merged commit 5be0397 into godotengine:master Jan 10, 2025
20 checks passed
@akien-mga
Copy link
Member

Thanks!

@Nazarwadim Nazarwadim deleted the use_AHashMap_in_SurfaceTool branch January 11, 2025 10:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants