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

Faster OTC font loading #1

Merged
merged 7 commits into from
Jan 15, 2024

Conversation

osiewicz
Copy link
Contributor

@osiewicz osiewicz commented Jan 15, 2024

This commit vastly simplifies the way .otc parsing is done in font-kit by initializing a font from a descriptor and not copying the buffer. It also makes Handle capable of storing an already loaded font, as what happens currently is that we first have a Font which we turn into a Handle (losing track of the Font that was used to create it) only to recreate a font from that same handle (and reparsing the font file in the process). Less waste is good.

I've had another stab at it over the weekend at https://github.com/osiewicz/font-kit/pull/new/improve_font_parsing_perf but changes from this PR are way smaller and make more sense IMHO. improve_font_parsing_perf tried to deal with issues in font-kit's implementation of otc parsing (it had an accidental O(n^2)), but it couldn't deal with the fact that font-kit had to allocate about 16Gb of memory (kid you not) to parse Iosevka file. :))))) Each 300Mb allocation took about 30ms, and we had to make 54 of those. Quick math: by allocations alone that approach couldn't get below 1.5s for parsing Iosevka, no matter how hard we tried.

Meanwhile, on this branch parsing Iosevka takes about 30ms and most of that time is spent.. well.. parsing the font I guess?
image

I want to let this code sit on Nightly a day or two before releasing it to the Preview.

…ad' if possible.

This fixes another chokepoint in Iosevka loading, as we were needlessly reading fonts from disk and stuff with load, whereas we've already had a font at our disposal before creating a handle. That seemed wasteful.
Note that I did cargo check --all-features this code, but I am only sure of coretext part compiling. If we pursue cross-platform support, we'll need to revisit this (probably). The idea from this commit is not specific to this commit though.
Instead, require Sync + Send for Handle::from_native
@osiewicz osiewicz merged commit d97147f into zed-industries:master Jan 15, 2024
2 of 3 checks passed
osiewicz added a commit to zed-industries/zed that referenced this pull request Jan 15, 2024
Details are in zed-industries/font-kit#1; We're
not doing anything too fancy, really. Still, you should mostly see font
loading times drop significantly for font collections
Release Notes:
- Improved loading performance of large font collections (e.g. Iosevka).
Fixes https://github.com/zed-industries/community/issues/1745,
https://github.com/zed-industries/community/issues/246


https://github.com/zed-industries/zed/assets/24362066/f70edbad-ded6-4c12-9c6d-7a487f330a1b
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant