Skip to content

Fix HVAR/VVAR performance regression from eager metric computation#1708

Merged
anthrotype merged 1 commit into
mainfrom
hvar-perf-regression
Oct 21, 2025
Merged

Fix HVAR/VVAR performance regression from eager metric computation#1708
anthrotype merged 1 commit into
mainfrom
hvar-perf-regression

Conversation

@anthrotype

Copy link
Copy Markdown
Member

The VVAR implementation in #1666 pre-computed GlobalMetricsInstance for all glyph locations upfront, causing thousands of unnecessary interpolations for sparse fonts. HVAR doesn't need metrics at all (only width), and VVAR was even recomputing the same location's metrics once per glyph, instead of once per unique glyph location.

We now compute metrics on-demand for VVAR only and cache per unique location.

Fixes #1706

@anthrotype

anthrotype commented Oct 21, 2025

Copy link
Copy Markdown
Member Author

fontc v0.3.2 still ran 1.58 ± 0.05 times faster (10s vs 16s) than fontc built from this PR according to hyperfine, but before this it was like 7x faster on my machine

The VVAR refactoring pre-computed GlobalMetricsInstance for all locations
upfront, causing thousands of unnecessary interpolations for sparse fonts.
HVAR doesn't need metrics at all (only width), and VVAR was recomputing
the same location's metrics once per glyph.

We now compute metrics on-demand for VVAR only and cache per unique location.

Fixes #1706
@anthrotype anthrotype force-pushed the hvar-perf-regression branch from 577e1d0 to cdb3beb Compare October 21, 2025 13:14
@anthrotype

Copy link
Copy Markdown
Member Author

for testing, I have also branched off the bisected commit and cherry-picked this PR's commit on top (only fixing a few conflicts) -- I pushed it here if you want to check:

https://github.com/googlefonts/fontc/compare/perf-test-isolated

When I run hyperfine off of that branch and compare with v0.3.2 (from wheel), I get basically the same time.

So while there may be other things that we added later on main that we could optimize, this fixes the major one so it's good to merge.

@cmyr

cmyr commented Oct 21, 2025

Copy link
Copy Markdown
Member

Will reiterate that it will be nice to set up some ongoing performance tracking at some point, would be great to catch this sort of thing in CI.

@cmyr cmyr left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

looks good!

@anthrotype anthrotype added this pull request to the merge queue Oct 21, 2025
Merged via the queue into main with commit 4c204ca Oct 21, 2025
12 checks passed
@anthrotype anthrotype deleted the hvar-perf-regression branch October 21, 2025 14:52
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.

Performance regression for compiling from Designspace in commit 20eb45d39debb699abf5126db175ef66a5d0ad73

2 participants