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

Improve performance of the 'Create New Node' dialog #86447

Merged
merged 1 commit into from
May 15, 2024

Conversation

Maran23
Copy link
Contributor

@Maran23 Maran23 commented Dec 22, 2023

Fixes: #27333
Fixes: #34205

  • Merged 3 for loops into 1 and save Vector allocation
    • We only iterate once over the type_list and add the matching types to the tree
  • Use get_instance_base_type() or get_global_name() instead of get_language()->get_global_class_name() for performance considerations
    • Behaviour is the same as before, but this is a simple lookup instead
  • Use StringName where appropriate
    • As the dialog consist of all Godot nodes, like Node, Node2D, Sprite2D, ... so using StringName makes sense here (and was also in place before, just not consistently)
  • Deduplicated the code in _add_type
    • The two if branches were nearly the same and could be merged easily. They benefit from the optimzation mentioned in the second point

This also fixes a bug where scripts/resources with compile errors were displayed in the dialog although they can not be used, resulting in errors (and even a crash) when searching in the dialog (Broken.gd is a file with compile errors):

Cannot get class 'Broken'.
editor\create_dialog.cpp:275 - Condition "inherits.is_empty()" is true.

Benchmarks:

Dialog with nodes

Tested in Debug Build
Test case: 800+ Global Classes

What Old New
Open Dialog (3x) 4340 ms, 1505 ms, 1521 ms 3693 ms, 905 ms, 868 ms
Search (Each letter of Node) 1589 ms, 399 ms, 402 ms, 401 ms 1036 ms, 260 ms, 258 ms, 247 ms
Search (Each letter of Box) 1196 ms, 1187 ms, 1185 ms 801 ms, 784 ms, 762 ms

Dialog with resources

Tested in Debug Build
Test case: 800+ Global Resources

What Old New
Open Dialog (3x) 2782 ms, 1043 ms, 1026 ms 2465 ms, 713 ms, 711 ms
Search (Each letter of Resource) 594 ms, 591 ms, 548 ms, 529 ms, 520ms, 520ms, 519ms, 522ms 441 ms, 427 ms, 391 ms, 381 ms, 373 ms, 369 ms, 366 ms, 369 ms
Search (Each letter of Custom) 578 ms, 541 ms, 525 ms, 524 ms, 523 ms, 521 ms 426 ms, 385 ms, 367 ms, 364 ms, 365 ms, 369 ms

Result

Performance improved between x1.2 and x1.6. Dialog feels more snappy.

Project:
ManyFiles.zip

Note

Similar improvements can be done to other places in the editor as far as I can see.

editor/create_dialog.cpp Outdated Show resolved Hide resolved
Copy link
Member

@Calinou Calinou left a comment

Choose a reason for hiding this comment

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

Tested locally (rebased on top of master ce00392), it works as expected.

With an optimized editor build, I can't notice too much a difference before and after this PR.

However, with a debug editor build, the difference is much more noticeable. The Create New Node dialog opens significantly faster with this PR and it doesn't freeze for as long when typing.

The difference should be even more noticeable on slower CPUs.

PC specifications
  • CPU: Intel Core i9-13900K
  • GPU: NVIDIA GeForce RTX 4090
  • RAM: 64 GB (2×32 GB DDR5-5800 C30)
  • SSD: Solidigm P44 Pro 2 TB
  • OS: Linux (Fedora 39)

editor/create_dialog.h Outdated Show resolved Hide resolved
editor/create_dialog.h Outdated Show resolved Hide resolved
editor/create_dialog.cpp Outdated Show resolved Hide resolved
editor/create_dialog.cpp Outdated Show resolved Hide resolved
- Merged 3 for loops into 1 and save Vector allocation
- Use get_instance_base_type() or get_global_name() instead of get_language()->get_global_class_name() for performance considerations
- Use StringName where appropriate
@Maran23 Maran23 force-pushed the performance-create-new-dialog branch from b6154c2 to af6a4f3 Compare May 15, 2024 18:50
@akien-mga akien-mga modified the milestones: 4.x, 4.3 May 15, 2024
@akien-mga akien-mga merged commit 2749645 into godotengine:master May 15, 2024
16 checks passed
@akien-mga
Copy link
Member

Thanks!

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