-
-
Notifications
You must be signed in to change notification settings - Fork 21.7k
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
Optimize text rendering by caching UBreakIterator
instances.
#102129
Open
Ivorforce
wants to merge
1
commit into
godotengine:master
Choose a base branch
from
Ivorforce:optimize-text-server-adv-break-iter
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Optimize text rendering by caching UBreakIterator
instances.
#102129
Ivorforce
wants to merge
1
commit into
godotengine:master
from
Ivorforce:optimize-text-server-adv-break-iter
+23
−3
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
e099a73
to
cb90204
Compare
Calinou
approved these changes
Jan 29, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested locally, it works as expected. Code looks good to me.
Benchmark
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 41)
Using an optimized editor build with LTO.
Startup + shutdown times of the editor on an empty project:
❯ hyperfine -iw1 -m50 "bin/godot.linuxbsd.editor.x86_64 /tmp/5/project.godot --quit" "bin/godot.linuxbsd.editor.x86_64.master /tmp/5/project.godot --quit"
Benchmark 1: bin/godot.linuxbsd.editor.x86_64 /tmp/5/project.godot --quit
Time (mean ± σ): 4.528 s ± 0.435 s [User: 2.589 s, System: 0.712 s]
Range (min … max): 3.770 s … 5.322 s 50 runs
Benchmark 2: bin/godot.linuxbsd.editor.x86_64.master /tmp/5/project.godot --quit
Time (mean ± σ): 4.620 s ± 0.340 s [User: 2.597 s, System: 0.693 s]
Range (min … max): 3.753 s … 4.825 s 50 runs
This PR is on average 0.1s faster to startup and shut down the editor on an empty project.
cb90204
to
6c82cf0
Compare
bruvzg
approved these changes
Feb 5, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Font
calculations by avoiding unnecessary copy-on-write. #102132.Today I profiled Godot launch times again.
Through caching, I was able to shave 4% total launch time off the Godot Editor (and additionally, improve render time of texts).
I am not familiar with the advanced text server, so I this change should be well tested and reviewed.
Explanation
It appears that
ubrk_open
converts its input locale from UTF8 to UTF16, allocating memory. This is surprisingly slow, and called quite often too.The implementation of
BreakIterator::createLineInstance(Locale(locale), *status);
appears to not be able to avoid this behavior, e.g. by usingUTF32
strings directly.Instead, it is easy to cache
UBreakIterator
instances. There is already a thread lock onsd->mutex
, so we should be safe that this does not cause cache variable contention.Using a
UBreakIterator
pool independent ofsd
might be even better; I experimented with this and got the launch time down by almost 12% (almost the entire contribution ofcreateLineInstance
). However, I am not aware of a precedent for variable pools like this, so that may require a more complex solution.Profiling details
I used Apple Instruments' CPU Profiler and inverted the call tree.
Launch
Before update
After update
Caveats
This change is likely to use some additional RAM. I don't have a sophisticated way to measure this, but my system reports a potential increase of 70mb:
It would be good if this could be measured more precisely to weigh it against the performance improvement.
Using a
UBreakIterator
pool (explained above) would likely further improve RAM efficiency.