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

Add thread support to CreateDialog #88984

Closed

Conversation

adamscott
Copy link
Member

@adamscott adamscott commented Feb 28, 2024

Adds thread support to CreateDialog. This removes will remove any engine-wide lag when searching in the CreateDialog.

Capture.video.du.2024-02-28.15-47-45.webm

(it has some glitches, see the note below about "Draft")

Notes

  • This takes into account THREADS_ENABLED, so it should work as well on builds without threads support.

Draft

  • This is in an early stage, so a lot will change before it being ready for production.
  • Will need to refactor a lot the code, as it is pretty ugly as of 2024-02-28.

Fixes

@KoBeWi

This comment was marked as resolved.

@adamscott

This comment was marked as resolved.

@adamscott
Copy link
Member Author

So it shows some issues right now with 1000 classes, I'll investigate why the threading doesn't help.

// Configure search items.
callable_mp(dialog, &CreateDialog::_create_search_option_items).bind(candidates, search_text).call_deferred();

while (true) {
Copy link
Member

Choose a reason for hiding this comment

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

This sort of busy loop is detrimental to performance. I'd suggest you to use a semaphore or a condition variable instead.

Copy link
Member

@RandomShaper RandomShaper left a comment

Choose a reason for hiding this comment

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

The trend is to have as many threaded tasks as possible in the WorkerThreadPool. I'd suggest using it instead of spawning bespoke threads. Besides the reason of having all the threaded work flowing through a single component that may do more clever scheduling in the future, bear in mind that creating a thread is not guaranteed to happen quickly. It's indeed somewhat slow on Windows.

@adamscott
Copy link
Member Author

Closing. I'll investigate more the issue.

@adamscott adamscott closed this Feb 29, 2024
@AThousandShips AThousandShips removed this from the 4.3 milestone Feb 29, 2024
@Maran23
Copy link
Contributor

Maran23 commented Mar 7, 2024

One note here: I did some small but still somewhat noticable improvements for the CreateDialog here #86447
This may or may not be at least a quick win to improve the performance a little bit.
Your Debouncing PR as well as improving the rendering of the Tree, maybe even Thread support are also good ideas to improve this further!

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