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

Restore fine-grained glyph work dependencies #380

Merged
merged 1 commit into from
Aug 10, 2023
Merged

Restore fine-grained glyph work dependencies #380

merged 1 commit into from
Aug 10, 2023

Conversation

rsheeter
Copy link
Contributor

@rsheeter rsheeter commented Aug 8, 2023

#359 changed BE glyph work from depending on only the IR glyphs used to all IR glyphs. This forces the can run check to take a slow path (table scan) due to use of Access::Custom and inhibits concurrent execution of FE and BE glyphs.

This PR late-binds dependencies for BE glyph work. Initially access unknown - never runnable - is used and then, once the corresponding IR glyph is done, a set of the specific dependencies is used. This should be faster to evaluate runnable on and re-enable concurrent FE/BE glyph work executions.

Intended to partially address #369.

#369 doesn't offer complete steps to reproduce but it appears to be roughly time fontc GS designspace. My tests and results follow (some noise from cli omitted, measured on an M1 Mac):

Oswald 2.415s => 1.302s

$ for br in conc main; do git checkout $br && cargo build --release && rm -rf build/ && time target/release/fontc --source ../OswaldFont/sources/Oswald.glyphs --emit-ir false; done

Switched to branch 'conc'

real	0m2.451s
user	0m0.104s
sys	0m0.019s
Switched to branch 'main'

real	0m1.302s
user	0m0.212s
sys	0m0.026s

Fixes #369 per #380 (comment)

@rsheeter
Copy link
Contributor Author

rsheeter commented Aug 8, 2023

Cloning Google Sans on airplane wifi is not going particularly well, hopefully the Oswald results will suffice for now.

@anthrotype
Copy link
Member

anthrotype commented Aug 8, 2023

I just tried to build GS and with this patched, it goes down from 25 seconds to 5!

EDIT: I used my PR branch #385 as base, made a new branch off that and merged your conc branch to it, then built GS with both and compared

@anthrotype anthrotype merged commit ee8d100 into main Aug 10, 2023
6 checks passed
@anthrotype anthrotype deleted the conc branch August 10, 2023 12:38
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.

Investigate performance regression
2 participants