LT-22674: Cache text analysis only for text NFC leaves unchanged

[jasonleenaylor]

Opening a text whose baseline is one long decomposed paragraph took minutes
in 9.3.9 and later. On the LT-22796 project it now takes 15 s on a Debug
build, against 18 s with the analysis cache disabled and 20 s on 9.3.7-era
code.

Start here: Src/views/lib/NfcOffsetMap.h, then CallScriptItemize in
Src/views/lib/UniscribeSegment.cpp. Then TestNfcOffsetMap in
Src/views/Test/TestViewCaches.h.

What it does

Opening or editing a long decomposed paragraph renormalized the text once per
analysed range. BuildNfcOffsetMaps in Src/views/lib/UniscribeSegment.cpp
built an offset map for the #724 analysis cache by NFC-normalizing every prefix
of the range, and UniscribeEngine::FindBreakPoint handed it the rest of the
paragraph on every line. That function is deleted.

NfcOffsetMap (Src/views/lib/NfcOffsetMap.h) replaces it with one table per
paragraph layout pass. LayoutPassCache holds the map and ParaBuilder resets
the cache when it starts laying out a paragraph, so the text is fetched and
normalized once per pass. The map splits the text where ICU's
hasBoundaryBefore says normalization cannot cross, records the NFC length
before each boundary, and answers a request inside a chunk by normalizing only
that chunk's tail. TryOffsetToOrig walks a chunk to the last offset whose
NFC form still fits, because a mark that reorders ahead of another can compose
with the base and shorten the form. A request whose base is not a boundary is
declined, and UniscribeSegment falls back to its on-demand translation.

TextAnalysisCache stores decomposed text again, with the NFC length and a
flag saying whether NFC changed it. CallScriptItemize serves a shorter
request from a longer decomposed entry only when the request ends on a
boundary, and otherwise recomputes.

TestViewCaches.h is now collected, so its dormant suites run, and gains
TestNfcOffsetMap, which checks every (base, offset) pair in both directions
against from-scratch normalization over a 12-string corpus. Three render
scenarios built from decomposed text join RenderBenchmarkTestsBase with
their own baselines.

Where to look

• Map correctness: TestNfcOffsetMap checks every (base, offset) pair in
  both directions against from-scratch normalization over a 12-string
  corpus (mark reordering, marks that reorder and compose, marks without
  base, Arabic, Hangul jamo, surrogates, lengthening and singleton
  decompositions).
• A shorter request served from a longer decomposed entry is only valid when
  it ends on a boundary; CallScriptItemize checks that and otherwise
  recomputes.
• TestViewCaches.h was never collected; its 9 tests run now, plus 7 new.
• Three render scenarios built from decomposed text (runtime ICU NFD,
  asserted decomposed). New baselines; the 15 existing ones are
  pixel-identical.

What is deliberately not here

Analysing a paragraph once and slicing per line: itemization was 7 of 90
layout samples against 27 for offset translation, and slicing needs a proof
about Uniscribe suffix itemization. Stays with LT-22656.

Verification

Build with -CommentHygiene -TokenHygiene clean. Native Views tests 318/318
after the last-fit walk; the new corpus string fails without it
(OffsetToOrig(5, 0) expected 9, got 7).
RootSiteTests render verify + timing 36/36, also green with each cache flag
off. Debug suite average cold render 225 ms default, 229 ms analysis cache
off, 289 ms shape cache off; the decomposed wrap scenario is 222 ms cached vs
323 ms not. Layout-time samples in offset translation: 27 of 90 with
BuildNfcOffsetMaps deleted and on-demand translation, 5 of 74 with the
table. Full managed suite not run.

Preflight review details

Code Review Summary

Branch: LT-22674-nfc-analysis-cache
Base: main
Date: 2026-09-28
Review model: Claude Fable 5.1
Files changed: 13

Overview

Purpose (author): stop the quadratic NFC offset-map build that #724 added to
the Uniscribe analysis cache, which made a 30,000-character decomposed
paragraph take minutes to lay out (LT-22674, LT-22796), without giving up
what #724 bought. Two commits, reviewable separately:

1. Restrict the analysis cache to text that NFC leaves unchanged and delete
   the map machinery. Restores pre-perf: Views engine render optimizations — warm 99.99% faster, cold 10% faster #724 behavior for decomposed text.
2. Add NfcOffsetMap: one table per paragraph layout pass, split at ICU
   normalization boundaries, giving every offset translation in one short
   normalization. Decomposed text is admitted to the analysis cache again,
   with shorter requests served only when they end on a boundary.

No Critical or Important findings. Rendering is pixel-identical to main on
all 15 pre-existing baselines after both commits.

Contract/API Changes

• TextAnalysisEntry / TextAnalysisCache::Store (Src/views/lib/LayoutCache.h,
  internal to Views.dll): commit 1 removed the offset-map fields and
  per-entry offset methods and renamed m_vchNfc to m_vchText; commit 2
  keeps m_fTextIsNfc and passes the NFC length and flag to Store.
• LayoutPassCache gains NfcOffsets(IVwTextSource*).
• UniscribeSegment: overloads taking TextAnalysisEntry* removed;
  CallScriptItemize lost ppAnalysis; new static CurrentNfcOffsetMap.
• No COM, IDL, manifest or exported-surface change.

Findings

Critical - Must address before merge

None.

Important - Should address before merge

None.

Minor - Consider

• Src/views/lib/UniscribeSegment.cpp: the on-demand OffsetToOrig
  starts its search at ich and so can overshoot for characters whose NFC
  is longer than the source (for example U+0344). The map's
  TryOffsetToOrig returns the definition's answer instead. The
  equivalence tests use from-scratch normalization as the oracle, so this
  is a correction, not a divergence; noted so nobody expects the fallback
  and the map to agree on such text.
• Src/views/Views.mak: no header dependency tracking. Adding a member
  to LayoutPassCache left VwTextBoxes.obj stale and produced Vector
  invariant asserts in VwParagraph.NormalizeNfd until the object was
  deleted. Pre-existing build defect (LT-22674 comment 2, item 3); flagged
  for anyone rebuilding this branch incrementally.

Positive Observations

• Map results are exact by construction: a prefix NFC length equals the
  recorded length before the nearest boundary plus the NFC length of the
  tail, and requests whose base is not a boundary decline so the caller
  falls back.
• 11-string corpus equivalence test (decomposed Latin, non-canonical mark
  order, marks without base, Arabic harakat, Hangul jamo, surrogate pairs,
  lengthening mark, singleton decomposition) checks every (base, offset)
  pair both directions against from-scratch normalization.
• Dormant TestViewCaches.h suites now collected; analysis-cache and
  store-rule tests added; three decomposed render scenarios built at
  runtime via ICU NFD with a decomposition assertion.

Required Validation / Evidence

• ./build.ps1 -CommentHygiene -TokenHygiene -- clean (both commits).
• ./test.ps1 -TestProject TestViews -SkipManaged -CommentHygiene -TokenHygiene
  -- 318 passed / 0 failed / 0 errors after commit 2 (315 after commit 1).
• RootSiteTests render verify + timing, default flags: 36/36; also
  18/18 with FW_PERF_P125_PATH2=0 and with FW_PERF_P125_PATH1=0.
• Debug-build suite averages, cold render: commit 1 -- 218 ms default,
  289 ms analysis cache off, 341 ms shape cache off; commit 2 -- 225 ms
  default, 229 ms analysis cache off, 289 ms shape cache off. Decomposed
  wrap scenario: 222 ms with the cache vs 323 ms without.
• Manual, reporter's project (one 30,117-char NFD paragraph), Debug
  build, time to responsive Baseline tab: before fix >10 min (killed);
  commit 1 ~20 s; commit 2 15.4 s / 16.0 s default vs 18.1 s / 18.2 s with
  the analysis cache off. Native stack samples of the layout portion:
  offset translation 27 of 90 samples after commit 1, 5 of 74 after
  commit 2.
• Full managed test suite not run.
• Jira: LT-22674 in branch name; LT-22796 linked "is solved by".

Interview Notes

• Author directed: one PR, two commits; commit 1 pushed as draft first.
• Author asked whether the change diverges from the NFD-in-memory,
  compose-on-output stance. No: NFC is a render-private copy for Uniscribe
  shaping that predates perf: Views engine render optimizations — warm 99.99% faster, cold 10% faster #724; offsets stay NFD; nothing is written back.
  Commit 2 removes the one gap commit 1 left (decomposed text without the
  cache benefit).
• Itemization slicing across lines (analysing a paragraph once and serving
  each line as a slice) was considered and left out: profiling after
  commit 1 showed itemization at 7 of 90 layout samples against 27 for
  offset translation, and slicing needs a proof that Uniscribe's
  itemization of a suffix equals the slice of the whole. LT-22656 scope.

In-Review Quality Check

No in-review changes.

Suggested Review Focus

• NfcOffsetMap::Build boundary logic and TryOffsetToOrig search.
• CallScriptItemize hit rule for decomposed entries (boundary-end).
• TestNfcOffsetMap::VerifyAgainstOracle corpus coverage.

🤖 Generated with Claude Code

---

This change is 

[github-actions]

NUnit Tests

    1 files  ± 0      1 suites  ±0   8m 56s ⏱️ - 3m 50s
6 319 tests +18  6 234 ✅ +18  85 💤 ±0  0 ❌ ±0 
6 328 runs  +18  6 243 ✅ +18  85 💤 ±0  0 ❌ ±0 

Results for commit d023698. ± Comparison against base commit b18c601.

♻️ This comment has been updated with latest results.

[codecov-commenter]

Codecov Report

❌ Patch coverage is 91.48936% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 39.02%. Comparing base (30564cc) to head (d023698).
⚠️ Report is 3 commits behind head on main.

Files with missing lines	Patch %	Lines
Src/views/lib/UniscribeEngine.cpp	83.33%	2 Missing ⚠️
Src/views/lib/UniscribeSegment.cpp	91.30%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1166      +/-   ##
==========================================
- Coverage   39.02%   39.02%   -0.01%     
==========================================
  Files        1522     1523       +1     
  Lines      352937   353018      +81     
  Branches    40726    40744      +18     
==========================================
+ Hits       137741   137762      +21     
- Misses     185893   185938      +45     
- Partials    29303    29318      +15     

Files with missing lines	Coverage Δ
Src/views/lib/LayoutCache.h	100.00% <100.00%> (ø)
Src/views/lib/NfcOffsetMap.h	100.00% <100.00%> (ø)
Src/views/lib/UniscribeSegment.h	92.10% <ø> (ø)
Src/views/lib/UniscribeEngine.cpp	70.71% <83.33%> (+0.92%)	⬆️
Src/views/lib/UniscribeSegment.cpp	70.33% <91.30%> (-0.46%)	⬇️

... and 18 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[johnml1135]

My reading of why commit 2 works when the ICU-boundary split was rejected on #1056. @jasonleenaylor, please approve or correct it.

LT-22674 comment 3 records that splitting at ICU normalization boundaries was tried for #1056 and rejected. The decomposed proxy still took 3,468 / 2,174 ms against a 298 / 269 ms control, because the linear build still ran once for every analysed range.

Commit 2 uses the same split, but LayoutPassCache::NfcOffsetsFor builds one table per paragraph layout pass. ParaBuilder resets that cache when a paragraph layout starts, so each character is normalized once per pass instead of once for every range that contains it. Your numbers fit this reading: the decomposed wrap scenario is 222 ms cached vs 323 ms uncached, and the LT-22796 project opens in 15 s vs 18 s.

I've recorded this on LT-22674 as unconfirmed. Correct it here or there.

[jasonleenaylor]

My reading of why commit 2 works when the ICU-boundary split was rejected on #1056.

That reading is right. The split at ICU boundaries is the same one #1056 tried; what changed is that ParaBuilder resets the layout pass cache when it starts a paragraph (VwTextBoxes.cpp:1200) and NfcOffsetsFor builds the table once per pass, so the linear build runs once per paragraph rather than once per analysed range. The one refinement: inside a chunk the tail is still normalized per request, which is what your first comment is about. Please mark it confirmed on LT-22674.

[johnml1135]

Approving. The one open review point is fixed, and CI is green.