[Text] Unicode and text layout optimizations - #21400
Conversation
There was a problem hiding this comment.
Pull request overview
This WIP PR focuses on improving text layout and Unicode processing performance by caching expensive per-cluster computations, reducing per-iteration overhead in line breaking/wrapping, and adding benchmarks/tests to lock in behavior and allocation characteristics.
Changes:
- Added a lazily-built cluster prefix-sum cache to
ShapedBufferand used it to speed up wrapping measurements and glyph-run width metrics. - Optimized
LineBreakEnumeratorby caching “next” class and combining several pure rules to reduce state accesses. - Added extensive new unit tests and benchmarks for Unicode tries/codepoint enumeration/break enumerators and text wrapping, plus a profiling harness for text layout.
Reviewed changes
Copilot reviewed 20 out of 21 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/Avalonia.Skia.UnitTests/Media/TextFormatting/TextLineTests.cs | Makes bidi bounds assertion tolerant to ULP-level float summation differences. |
| tests/Avalonia.Skia.UnitTests/Media/TextFormatting/TextFormatterWrapCharacterizationTests.cs | Adds characterization tests to pin wrapping behavior via TextFormatterImpl.FormatLine. |
| tests/Avalonia.Benchmarks/Text/UnicodeTrieBenchmark.cs | Adds benchmarks for raw UnicodeTrie.Get throughput across tries/distributions. |
| tests/Avalonia.Benchmarks/Text/UnicodeBreakEnumeratorBenchmark.cs | Adds benchmarks for line/word/grapheme break enumerator iteration cost. |
| tests/Avalonia.Benchmarks/Text/TextLayoutProfile.cs | Adds a single-case profiling-oriented benchmark using a shared TextRunCache. |
| tests/Avalonia.Benchmarks/Text/HugeTextLayout.cs | Exposes emoji fixture text for reuse and updates benchmark to use the renamed constant. |
| tests/Avalonia.Benchmarks/Text/CodepointBenchmark.cs | Adds benchmarks covering Codepoint.ReadAt, enumeration, and property access patterns. |
| tests/Avalonia.Benchmarks/Program.cs | Adds a non-BDN profiling mode (--profile-textlayout) that boots real Skia/HarfBuzz services. |
| tests/Avalonia.Base.UnitTests/Media/TextFormatting/UnicodeTrieTests.cs | Adds direct unit coverage for UnicodeTrie/UnicodeTrieBuilder branches and round-trips. |
| tests/Avalonia.Base.UnitTests/Media/TextFormatting/UnicodeEnumeratorAllocationTests.cs | Adds allocation guard tests for break enumerators using GC.GetAllocatedBytesForCurrentThread. |
| tests/Avalonia.Base.UnitTests/Media/TextFormatting/UnicodeDataTests.cs | Adds spot checks for UnicodeData public surface (masks/shifts/defaults/known codepoints). |
| tests/Avalonia.Base.UnitTests/Media/TextFormatting/PropertyValueAliasHelperTests.cs | Adds lightweight round-trip tests for generated alias helper mappings. |
| tests/Avalonia.Base.UnitTests/Media/TextFormatting/LineBreakEnumeratorTests.cs | Re-enables previously skipped line break test data. |
| tests/Avalonia.Base.UnitTests/Media/TextFormatting/CodepointTests.cs | Adds comprehensive Codepoint unit tests (surrogates, helpers, bracket pairing, etc.). |
| src/Skia/Avalonia.Skia/GlyphRunImpl.cs | Fuses glyph position and bounds accumulation to reduce redundant passes. |
| src/Avalonia.Base/Media/TextFormatting/Unicode/LineBreakPairTable.cs | Removes unused historical pair table implementation. |
| src/Avalonia.Base/Media/TextFormatting/Unicode/LineBreakEnumerator.cs | Adds cached NextClass/PreviousClass in state and combines several rules for speed. |
| src/Avalonia.Base/Media/TextFormatting/TextFormatterImpl.cs | Uses ShapedBuffer cluster cache for wrap measurement; refactors wrapping loop and SplitTextRuns to return split length. |
| src/Avalonia.Base/Media/TextFormatting/ShapedBuffer.cs | Introduces shared cluster prefix-sum cache (including across splits) and cache-backed measurement helpers. |
| src/Avalonia.Base/Media/GlyphRun.cs | Uses ShapedBuffer.TotalGlyphAdvance to avoid O(n) advance summation in metrics creation. |
| .gitignore | Ignores /planning. |
|
You can test this PR using the following package version. |
|
You can test this PR using the following package version. |
|
You can test this PR using the following package version. |
8bbb1cb to
3cf6919
Compare
- ShapedBuffer: lazy, pooled cluster-prefix cache shared across Split; adds TotalGlyphAdvance / MeasureCharactersThatFit. - GlyphRun/TextFormatterImpl/Skia GlyphRunImpl: read the cache, drop duplicate scans; SplitTextRuns now returns firstLength via out. - LineBreakEnumerator: cache Next/PreviousClass per advance; remove unused LineBreakPairTable. - Tests + benchmarks for the new paths and a dotnet-trace harness. ~1.32x faster / -5% alloc on the emoji-wrap micro-benchmark; no observable behaviour or public API change.
02923b8 to
ec614b7
Compare
Replace the BreakUnitDelegate[] s_rules array dispatch in
LineBreakEnumerator.ExecuteRules with a sequence of direct static
calls and a `goto Done` early-exit. This removes 42 indirect calls
per codepoint and lets the JIT reason across rule boundaries, which
in turn makes [MethodImpl(AggressiveInlining)] meaningful — JIT
cannot inline through delegate.Invoke, so the attribute was a no-op
in the previous shape.
Selectively apply AggressiveInlining to the small single-condition
rules (LB03, LB04, LB06, LB07, LB08a, LB11–LB15d, LB18, LB20,
LB21b, LB22, LB29, LB31) and let the JIT decide on the larger ones
to avoid bloating the merged ExecuteRules.
The static BreakUnitDelegate[] s_rules array is removed.
Benchmarks (BDN default job, --inProcess, N=13–22, rel. StdDev <1.5%):
UnicodeBreakEnumeratorBenchmark.LineBreakEnumerator_Sequence
Ascii 154.6 µs -> 24.84 µs (6.22x)
Bmp 190.2 µs -> 32.81 µs (5.80x)
Supplementary 227.6 µs -> 37.56 µs (6.06x)
TextLayoutProfile.BuildEmojisWrapped
Before (branch, pre-inline) 804.5 µs / 570.15 KB
After (branch, post-inline) 547.0 µs / 570.15 KB (-32.0%)
vs upstream/master (1061 µs) -> ~1.94x total speedup
No allocation change; CPU-only dispatch reshape. All
LineBreakEnumerator unit tests pass (5/5).
|
You can test this PR using the following package version. |
|
You can test this PR using the following package version. |
…nter Wrap the ArrayPool-rented glyph and cluster-cache arrays in a small PooledArray<T> disposable and expose them through IRef<T>. Split children and WithBidiLevel aliases now Clone() the refs instead of borrowing raw pool arrays, so the backing storage survives until every sibling has been disposed - eliminating the UAF risk that existed when a parent was disposed before its children. Add a per-glyph-holder generation counter (Volatile.Read / Interlocked .Increment). The indexer setter bumps the counter on every write, and EnsureClusterCache compares its recorded generation against the holder's current value, rebuilding on mismatch. This lets us drop the previous "no mutation after Split/WithBidiLevel" contract: mutations performed through any sibling now propagate to the others' caches transparently. Dispose is made idempotent via a _disposed guard so overlapping cache eviction and TextLine teardown only release the IRefs once. Adds ShapedBufferSharedStorageTests covering sibling lifetime, Dispose idempotency, and generation-driven cache invalidation across Split children and WithBidiLevel aliases.
|
You can test this PR using the following package version. |
…Level aliases Add a regression test hook (`ClusterPrefix`) exposing the backing cluster-prefix array reference, plus two tests that mutate a parent buffer before aliasing it and assert the alias reuses the parent's pooled prefix array instead of rebuilding. Guards against forgetting to propagate `_cacheGeneration` to alias buffers, which would silently defeat the cached-split fast path.
|
You can test this PR using the following package version. |
MrJul
left a comment
There was a problem hiding this comment.
Impressive numbers, LGTM, let's merge!
|
You can test this PR using the following package version. |
* perf(text): O(1) width queries on ShapedBuffer, cached LB-class lookup
- ShapedBuffer: lazy, pooled cluster-prefix cache shared across Split;
adds TotalGlyphAdvance / MeasureCharactersThatFit.
- GlyphRun/TextFormatterImpl/Skia GlyphRunImpl: read the cache, drop
duplicate scans; SplitTextRuns now returns firstLength via out.
- LineBreakEnumerator: cache Next/PreviousClass per advance; remove
unused LineBreakPairTable.
- Tests + benchmarks for the new paths and a dotnet-trace harness.
~1.32x faster / -5% alloc on the emoji-wrap micro-benchmark; no
observable behaviour or public API change.
* perf(TextFormatting): inline LineBreakEnumerator rule dispatch
Replace the BreakUnitDelegate[] s_rules array dispatch in
LineBreakEnumerator.ExecuteRules with a sequence of direct static
calls and a `goto Done` early-exit. This removes 42 indirect calls
per codepoint and lets the JIT reason across rule boundaries, which
in turn makes [MethodImpl(AggressiveInlining)] meaningful — JIT
cannot inline through delegate.Invoke, so the attribute was a no-op
in the previous shape.
Selectively apply AggressiveInlining to the small single-condition
rules (LB03, LB04, LB06, LB07, LB08a, LB11–LB15d, LB18, LB20,
LB21b, LB22, LB29, LB31) and let the JIT decide on the larger ones
to avoid bloating the merged ExecuteRules.
The static BreakUnitDelegate[] s_rules array is removed.
Benchmarks (BDN default job, --inProcess, N=13–22, rel. StdDev <1.5%):
UnicodeBreakEnumeratorBenchmark.LineBreakEnumerator_Sequence
Ascii 154.6 µs -> 24.84 µs (6.22x)
Bmp 190.2 µs -> 32.81 µs (5.80x)
Supplementary 227.6 µs -> 37.56 µs (6.06x)
TextLayoutProfile.BuildEmojisWrapped
Before (branch, pre-inline) 804.5 µs / 570.15 KB
After (branch, post-inline) 547.0 µs / 570.15 KB (-32.0%)
vs upstream/master (1061 µs) -> ~1.94x total speedup
No allocation change; CPU-only dispatch reshape. All
LineBreakEnumerator unit tests pass (5/5).
* Refactor ShapedBuffer to share pool storage via IRef + generation counter
Wrap the ArrayPool-rented glyph and cluster-cache arrays in a small
PooledArray<T> disposable and expose them through IRef<T>. Split children
and WithBidiLevel aliases now Clone() the refs instead of borrowing raw
pool arrays, so the backing storage survives until every sibling has
been disposed - eliminating the UAF risk that existed when a parent was
disposed before its children.
Add a per-glyph-holder generation counter (Volatile.Read / Interlocked
.Increment). The indexer setter bumps the counter on every write, and
EnsureClusterCache compares its recorded generation against the holder's
current value, rebuilding on mismatch. This lets us drop the previous
"no mutation after Split/WithBidiLevel" contract: mutations performed
through any sibling now propagate to the others' caches transparently.
Dispose is made idempotent via a _disposed guard so overlapping cache
eviction and TextLine teardown only release the IRefs once.
Adds ShapedBufferSharedStorageTests covering sibling lifetime,
Dispose idempotency, and generation-driven cache invalidation across
Split children and WithBidiLevel aliases.
* test(ShapedBuffer): cover cluster-cache sharing across Split/WithBidiLevel aliases
Add a regression test hook (`ClusterPrefix`) exposing the
backing cluster-prefix array reference, plus two tests that mutate a
parent buffer before aliasing it and assert the alias reuses the
parent's pooled prefix array instead of rebuilding. Guards against
forgetting to propagate `_cacheGeneration` to alias buffers, which
would silently defeat the cached-split fast path.
* Correctly use MathUtilities.LessThanOrClose
---------
Co-authored-by: Julien Lebosquain <julien@lebosquain.net>
What does the pull request do?
Performance + correctness pass on the text formatting pipeline. Wrap, measure and metrics paths now read width data from a cached cluster-prefix sum on
ShapedBufferinstead of walking glyphs every time, theLineBreakEnumeratorstops re-decoding the same codepoint on each rule and now dispatches its rules through direct static calls + selective inlining (no more delegate array), and the SkiaGlyphRunImplbuilds positions and ink bounds in a single fused pass.What is the current behavior?
TextLayoutre-sums per-glyph advances on everyMeasureLength, total-width, and split-driven wrap iteration.SplitTextRunsneeds a second pass to total the first half's length.LineBreakEnumeratorrules callstate.Next(text).LineBreakClassrepeatedly, redoing the trie lookup per rule, and dispatch each of its 42 rules through aBreakUnitDelegate[]— the indirect call blocks JIT inlining across rule boundaries.GlyphRunImplwalks the glyph array twice with separatecurrentXaccumulators and returns the rentedSKRect[]outside of atry/finally.What is the updated/expected behavior with this PR?
Identical observable behaviour, faster steady-state.
LineBreakEnumeratormicro-bench (1024 codepoints, BDN default job,--inProcess, same-session vsupstream/master):End-to-end
TextLayoutProfile.BuildEmojisWrapped(worst-caseWrapWithOverflow+ emoji input, same-session pair, BDN default job,--inProcess):≈ 1.28× end-to-end speedup, no Gen1 regression. New characterisation tests pin wrap and split-runs behaviour so the refactors stay observationally equivalent.
How was the solution implemented (if it's not obvious)?
ShapedBuffergrows a lazily built cluster-prefix-sum cache (advances + optional start-char offsets), pooled viaArrayPool, shared by reference acrossSplithalves andWithBidiLevelaliases through a private offset-aware constructor.A fast path skips the start-chars array when every cluster is exactly one char wide. A DEBUG-only
_cacheSharedguard on the indexer setter catches post-split mutation.GlyphRun.CreateGlyphRunMetricsand the wrap path readTotalGlyphAdvance/MeasureCharactersThatFitin O(1);SplitTextRunsexposes afirstLengthoutparameter (old signature preserved) so the wrap caller drops its second sweep.LineBreakEnumeratorcachesNextClass/PreviousClassin advance, and all LB rules read those fields; itsExecuteRuleswas rewritten from aBreakUnitDelegate[]loop to direct sequential static calls withgoto Doneearly-exit, and the small single-condition rules are tagged[MethodImpl(AggressiveInlining)](the attribute was a no-op through the delegate array and only takes effect now). The unusedLineBreakPairTable.csis removed. SkiaGlyphRunImplfuses the twocurrentXwalks into one and moves the pool return intofinally.Checklist
Breaking changes
None. Public API surface is unchanged; the new
SplitTextRunsoverload is internal, and the old signature still exists as a thin wrapper.Obsoletions / Deprecations
None.
Fixed issues