Skip to content

Cut allocations out of the shaping hot paths - #291

Merged
benoitkugler merged 5 commits into
go-text:mainfrom
egonelbre:perf/harfbuzz
Oct 1, 2026
Merged

Cut allocations out of the shaping hot paths#291
benoitkugler merged 5 commits into
go-text:mainfrom
egonelbre:perf/harfbuzz

Conversation

@egonelbre

@egonelbre egonelbre commented Sep 29, 2026

Copy link
Copy Markdown
Contributor

Six commits, each one allocation site or one missed cache. Numbers are benchstat on the existing BenchmarkShaping runs, n=6, Apple M4 Max, first commit versus last.

  • The Arabic fallback plan was rebuilt on every Shape because the shaper copied its plan struct by value and stored the result in the copy. Fonts without init/medi/fina rules paid for cmap lookups and a sort each call. Only the en-words Roboto run hit this path.
  • Context and ChainedContext matchers were closures capturing the class table or coverage list, one heap escape per candidate glyph. They are now a small struct passed by value.
  • Nested lookup recursion allocated its match position array and boxed the lookup into an interface on every level. The apply context now carries a per-depth array and the recursion takes a pointer into the face's lookup slice.
  • Pair positioning parsed both value records into fresh slices at every bisection step and only used the second glyph. It now reads the glyph while bisecting and parses the record once on a hit, into a fixed array.
  • Mark attachment re-parsed and boxed the base, ligature, and mark2 anchor at every attachment. The anchor matrix now hands back the raw table bytes and the three format parsers are shared with the pre-parsed mark side.
  • The shape plan cache key was built by running the full plan init, including GSUB and GPOS variation index lookups, before the cache was even consulted. The two compared fields are now set directly.

End to end, per Shape:

run sec/op before after allocs/op before after
en-words, Roboto 41.6ms 25.1ms 251k 6
en-thelittleprince, Roboto 28.1ms 21.9ms 275k 5
fa-thelittleprince, Amiri 42.2ms 37.5ms 1.11M 7
fa-monologue, Amiri 7.44ms 6.34ms 188k 3
fa-thelittleprince, Nastaliq 172ms 175ms 577k 27
fa-monologue, Nastaliq 30.8ms 31.6ms 98.7k 6

Nastaliq time does not move. Format 2 anchor points parse glyph outlines on every attachment, and that dominates its profile. This branch does not touch it.

@benoitkugler

benoitkugler commented Sep 30, 2026

Copy link
Copy Markdown
Contributor

This looks great, but I will need to take a closer look. In the meantime, it seems the last column of the benchmark results is broken (or at least missing a unit) ?

@egonelbre

egonelbre commented Sep 30, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Do you mean in the PR description? The last columns are "allocations after"; e.g. going from 251k allocations to 6 allocations.

@benoitkugler

benoitkugler commented Sep 30, 2026

Copy link
Copy Markdown
Contributor

Do you mean in the PR description? The last columns are "allocations after"; e.g. going from 251k allocations to 6 allocations.

Oh, alright, I did not imagine the allocations could be that low :)

benoitkugler
benoitkugler requested changes Oct 1, 2026
View reviewed changes

@benoitkugler benoitkugler left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'll like all the optimization, except the last one (commit 6691541) : it seems not to matter in practical uses cases, and to complexify (just a little bit) the code. Can we drop it ?

@egonelbre

egonelbre commented Oct 1, 2026

Copy link
Copy Markdown
Contributor Author

@benoitkugler sure, no problem in dropping it. I was a bit on the fence about it myself.

@egonelbre
[harfbuzz] cache the Arabic fallback plan on the shaper plan
3e19394 
arabicFallbackShape copied complexShaperArabic.plan by value, so the
fallback plan it built never reached the shaper plan. Every Shape of a
font without init/medi/fina rebuilt it, with its cmap lookups, sort and
accelerators. Take a pointer and store the built plan. The plan cache
belongs to one Buffer, so nothing shares it.

benchstat, n=6, Apple M4 Max. Only the run that hits the fallback moves.

                             │ before │ after  │
                             │ sec/op │ sec/op │
en-words.txt - Roboto          41.57m   30.17m  -27.44% (p=0.002)
en-thelittleprince - Roboto    28.13m   28.01m  ~
fa-thelittleprince - Amiri     42.16m   42.56m  ~
fa-thelittleprince - Nastaliq  172.2m   172.7m  ~
fa-monologue - Amiri           7.436m   7.544m  ~
fa-monologue - Nastaliq        30.77m   31.22m  ~

B/op for en-words drops from 2.494Mi to 2.132Mi, 14.5% less. allocs/op
is unchanged.
@egonelbre
[harfbuzz] build context matchers as values instead of closures
2ba9d89 
matchClass and matchCoverage returned closures that captured the
ClassDef or the coverage list. Each closure escaped to the heap on every
Context 2/3 and ChainedContext 2/3 apply, once per candidate glyph.
matcherFunc is now a plain struct that holds the class or the coverages
and matches by glyph ID when neither is set. otApplyContextMatcher
stores it by value and uses a hasMatchFunc flag where it checked for
nil.

benchstat vs previous commit, n=6, Apple M4 Max:

                             │ before │ after  │
                             │ sec/op │ sec/op │
fa-thelittleprince - Amiri     42.56m   37.51m  -11.87% (p=0.002)
fa-thelittleprince - Nastaliq  172.7m   177.3m   +2.70% (p=0.026)
fa-monologue - Amiri           7.544m   6.337m  -16.00% (p=0.002)
fa-monologue - Nastaliq        31.22m   31.67m   ~
en-thelittleprince - Roboto    28.01m   28.78m   +2.77% (p=0.002)
en-words - Roboto              30.17m   31.39m   +4.05% (p=0.002)

                             │ before    │ after   │
                             │ allocs/op │ allocs/op │
fa-thelittleprince - Amiri     1110.5k    37.76k  -96.60%
fa-thelittleprince - Nastaliq  577.2k     149.3k  -74.13%
fa-monologue - Amiri           187.9k     6.821k  -96.37%
fa-monologue - Nastaliq        98.73k     26.08k  -73.58%
en-thelittleprince - Roboto    275.2k     262.4k   -4.68%
en-words - Roboto              250.8k     233.8k   -6.77%

B/op: Amiri 35.8Mi -> 2.96Mi, Nastaliq 24.2Mi -> 14.4Mi, Roboto -20%.
Roboto loses 3-4% because the 40-byte matcher now travels by value
where a closure pointer did before. The later commits in this series
win that back.
@egonelbre
[harfbuzz] stop allocating on nested lookup recursion
a19df77 
applyRecurseLookup declared a [8]int for the nested match positions
and the array escaped to the heap on every recursion. otApplyContext
now carries a [maxNestingLevel][8]int indexed by nesting depth.
applyRecurseGSUB and applyRecurseGPOS also boxed the whole lookup
struct into the layoutLookup interface per recursion. They now pass a
pointer into the face's lookup slice.

benchstat vs previous commit, n=6, Apple M4 Max:

                             │ before    │ after     │
                             │ allocs/op │ allocs/op │
fa-thelittleprince - Amiri     37759      7        -99.98%
fa-thelittleprince - Nastaliq  149.3k     19.68k   -86.82%
fa-monologue - Amiri           6821       3        -99.96%
fa-monologue - Nastaliq        26.08k     3.278k   -87.43%
en-thelittleprince - Roboto    262.4k     262.4k    ~
en-words - Roboto              233.8k     233.8k    ~

                             │ before │ after  │
                             │ sec/op │ sec/op │
fa-thelittleprince - Amiri     37.51m   37.01m  -1.33% (p=0.009)
fa-thelittleprince - Nastaliq  177.3m   175.0m  -1.31% (p=0.002)
fa-monologue - Amiri           6.337m   6.263m  ~
fa-monologue - Nastaliq        31.67m   31.85m  ~
en-thelittleprince - Roboto    28.78m   29.95m  +4.06% (noisy run)
en-words - Roboto              31.39m   32.73m  +4.27% (±28%, noisy)

B/op: Amiri 2.84Mi -> 1.11Mi, Nastaliq 14.4Mi -> 8.5Mi. Roboto never
recurses and its allocs are unchanged, so its timing delta is machine
noise.
@egonelbre
[harfbuzz] read pair positioning value records without allocating
552fb9d 
parseValueRecord called ParseUint16s and allocated a slice per record.
A ValueFormat has at most 8 fields, so read them into a fixed array.
PairSet.FindGlyph parsed both value records on every bisection step and
only used SecondGlyph. It now reads the glyph alone while bisecting and
parses the record once on a hit. PairPosData2.Record goes through the
same parseValueRecord and gains the same fix.

benchstat vs previous commit, n=6, Apple M4 Max:

                             │ before    │ after     │
                             │ allocs/op │ allocs/op │
en-thelittleprince - Roboto    262363     5        -100.00%
en-words - Roboto              233804     6        -100.00%
fa-* (Amiri, Nastaliq)         unchanged (no PairPos kerning hit)

                             │ before │ after  │
                             │ sec/op │ sec/op │
en-thelittleprince - Roboto    29.95m   22.12m  -26.15% (p=0.002)
en-words - Roboto              32.73m   25.36m  -22.50% (p=0.002)
fa-thelittleprince - Amiri     37.01m   38.78m   +4.79% (noisy run)
fa-thelittleprince - Nastaliq  175.0m   177.6m   +1.51%
fa-monologue - Amiri           6.263m   6.354m   +1.46%
fa-monologue - Nastaliq        31.85m   32.11m   ~

B/op Roboto: 1581Ki -> 802Ki and 1.68Mi -> 1.03Mi.
@egonelbre
[harfbuzz] read base/ligature/mark2 anchors without boxing them
7bd50c0 
AnchorMatrix.Anchor re-parsed the anchor and boxed it into the Anchor
interface on every mark attachment. AnchorMatrix.AnchorBytes returns
the raw anchor table instead, or nil when there is none, and
applyGPOSMarks parses it with ParseAnchorFormat1/2/3. getAnchor1/2/3
hold the per-format code, so the pre-parsed mark anchors and the raw
path share it. AnchorMatrix.Anchor keeps its behavior.

benchstat vs previous commit, n=6, Apple M4 Max:

                             │ before    │ after     │
                             │ allocs/op │ allocs/op │
fa-thelittleprince - Nastaliq  19675      27       -99.86%
fa-monologue - Nastaliq        3277       6        -99.82%
fa-* Amiri, en-* Roboto        unchanged (3-8 allocs/op)

                             │ before │ after  │
                             │ sec/op │ sec/op │
fa-thelittleprince - Amiri     38.78m   37.45m  ~
fa-thelittleprince - Nastaliq  177.6m   175.3m  -1.30% (p=0.002)
fa-monologue - Amiri           6.354m   6.337m  ~
fa-monologue - Nastaliq        32.11m   31.62m  ~
en-thelittleprince - Roboto    22.12m   21.86m  -1.21% (p=0.004)
en-words - Roboto              25.36m   25.12m  ~

B/op Nastaliq: 8.505Mi -> 8.355Mi and 276.8Ki -> 255.2Ki. Glyph
outline parsing for format 2 anchor points dominates Nastaliq time, and
this change does not touch it.
benoitkugler
benoitkugler approved these changes Oct 1, 2026
View reviewed changes

@benoitkugler benoitkugler left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Great, thank you !

@benoitkugler
benoitkugler merged commit 64922d2 into go-text:main Oct 1, 2026
7 checks passed
@egonelbre
egonelbre deleted the perf/harfbuzz branch October 1, 2026 15:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@benoitkugler benoitkugler benoitkugler approved these changes
@whereswaldon whereswaldon Awaiting requested review from whereswaldon whereswaldon is a code owner
@andydotxyz andydotxyz Awaiting requested review from andydotxyz andydotxyz is a code owner

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@egonelbre @benoitkugler