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 dumb theme item cache to Control #64314

Merged
merged 1 commit into from
Aug 15, 2022

Conversation

YuriSizov
Copy link
Contributor

@YuriSizov YuriSizov commented Aug 12, 2022

I've experimented with this a year ago, and in targeted testing it didn't yield any meaningful performance improvements. But there seems to be a benefit to it still in some extreme situations, like mentioned in #64241. So I'm reviving it, as it shouldn't have any downsides and would be beneficial for cases that abuse theme item fetching without a cache on the user side.

Out of curiosity, I've retested the snippet from #64271 (review) again, and got a huge boost, on top of the caching added by @KoBeWi in that PR. From ~5.4s down to ~4.3s in debug builds! There are clearly caches missing in some of the editor log controls. Likely, RichTextLabel is doing something nasty.

This can be backported to 3.x, adjusted for the code structure there.

cc @AaronRecord since you're interested in optimizing theme.

@jordo
Copy link
Contributor

jordo commented Aug 12, 2022

From ~5.4s down to ~4.3s in debug builds!

Our benchmarks in 3.5 here #64329 on purely the affected get_* control functions show much more than a 20% improvement. Seeing 600% from a quick and dirty gdscript test in editor, and up to 20X speedup when called natively on release export builds.

@YuriSizov YuriSizov merged commit 4426049 into godotengine:master Aug 15, 2022
@YuriSizov YuriSizov deleted the control-theme-item-cache branch August 15, 2022 13:36
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.

3 participants