Stress-test DocxDiff on a heavyweight legal document, and stop it reading each file four times

[JSv4]

Why

We had no way to answer "how fast is DocxDiff on a real legal document, and where does the time go?" — and no way to prove a performance change left the diff itself untouched. This adds both, then acts on what they showed.

The reference document is the NVCA Model Certificate of Incorporation (October 2025): 574 KB of word/document.xml holding 15,360 elements across 234 body paragraphs, plus 97 footnotes in a 227 KB part, 16 abstract numbering definitions, 4 sections, 8 headers and 10 footers. A DocxDiff.Compare of it against an edited copy took about 820 ms.

What the measurement showed

About 72% of a comparison was spent inside IrReader, reading the same two documents four times.

1. DocxDiffComparison read both sides with RetainSources off to build the edit script.
2. IrMarkupRenderer then re-read the same two documents with RetainSources on, to get the source w:p/w:tbl elements it clones from.
3. And inside every one of those reads, settling the accepted-revision view opened a whole second package and parsed every story to scan it for revision markup — then threw that parse away and reopened the document for the walk.

None of it was necessary. RetainSources decides only whether IrProvenance pins the source XElement, and IrProvenance is equality-neutral by construction (it equals any other instance and hashes to zero, precisely so provenance never leaks into IR value equality). The two snapshots are therefore node-for-node value-equal, and the renderer can simply be handed the one the script was built over. And because GetXDocument caches per part, running the revision scan against the package the walk is about to use costs nothing.

The N-way path carried the same duplication multiplied by reviewer count: an N-reviewer Consolidate read 2*(N+1) packages to compare N+1 documents.

How it works now

• One read per document per comparison. IrMarkupRenderer.Render and IrCompositeMarkupRenderer.Render take an optional pre-read snapshot; DocxDiffComparison and Consolidate read once with provenance on and hand it over.
• One package open per read. IrReader.Read opens the package, decides the revision view against it, and walks it. Only a document that genuinely needs a RevisionProcessor round-trip reopens.
• Both sides concurrently, where there are threads to do it on — see the guard section below.
• GetRevisions on byte-identical packages returns empty immediately, the same shortcut ToRedline already had. The edit script keeps no such shortcut: its all-Equal operations are the answer the caller asked for.
• Cheaper hashing walks. UnidHelper.ContentSignature walked every element's subtree three times building a string per walk; one fused walk now appends the same characters in the same order, and a per-call cache serves its repeated hash inputs (about seven in eight are duplicates). Canonical-XML hashing no longer materializes a byte array per call, and attribute canonicalization skips the sort for elements carrying none or one.

Results

Medians of nine timed iterations after four warm-ups, before and after in one run:

Case	Before	After
8 scattered word edits	821 ms	351 ms	2.3×
an edit in every fifth paragraph	868 ms	395 ms	2.2×
24 blocks relocated	792 ms	338 ms	2.3×
20 paragraphs deleted, 20 inserted	803 ms	334 ms	2.4×
half the footnote paragraphs edited	959 ms	510 ms	1.9×
every second text node edited	1062 ms	611 ms	1.7×
every paragraph rewritten	1279 ms	868 ms	1.5×
GetRevisions	395 ms	216 ms	1.8×
GetEditScriptJson	389 ms	216 ms	1.8×
all three products fused	852 ms	387 ms	2.2×
GetRevisions, identical packages	371 ms	0 ms	—
Consolidate, 4 reviewers	1870 ms	675 ms	2.8×

Allocation per comparison drops 528 → 276 MB; per four-reviewer consolidate 1337 → 710 MB.

How it was validated

4,161 of 4,161 .NET tests pass, warning count byte-for-byte identical to main (262, same distribution), WASM builds at 3689 KB wire size (budget 4096), browser diff specs green.

The evidence that matters for a change like this is that the diff itself did not move, and it is checkable:

dotnet run -c Release --project benchmarks/docxdiff-stress -- --corpus TestFiles --baseline main.json   # on main
dotnet run -c Release --project benchmarks/docxdiff-stress -- --corpus TestFiles --check main.json      # on this branch

678 documents → 8,136 digests → all identical. Each document contributes an edited variant under default settings, the document against itself (the byte-identical shortcuts), and the edited variant again under PreAcceptInputRevisions and PreserveInputRevisions — the only settings that reach the revision-transform path — times three products.

That corpus matters because the reference document has no tracked revisions, no tables and no drawings, which is to say it never exercised the path IrReader.Read was restructured around. TestFiles has 89 revision-bearing, 218 table-bearing, 95 drawing-bearing and 124 content-control-bearing documents.

Validated in both directions, because a check that cannot fail proves nothing:

main vs this branch	all 8,136 identical
main vs itself	all 8,136 identical
this branch vs itself	all 8,136 identical
drop the w:t skip in UnidHelper.ContentSignature	3,905 mismatches

And its limit, stated rather than glossed: reintroducing the wp:docPr/@id stripping-order bug that IrHasherTests.Canonicalize_LoneDocPrId_StillStripped guards produces zero corpus mismatches — real Word documents emit docPr with both id and name, so the lone-attribute shape never occurs in these 678 files. Corpus parity and unit tests cover different things.

Found on the way: Compare is not byte-deterministic with media (#621)

The first corpus run reported 161 mismatches. They were not this branch's: main disagrees with itself on exactly the same 161. Media and diagram parts imported into a redline are named P + a fresh GUID, with relationship ids of R + a fresh GUID, so any document whose redline imports media produces different bytes every run — contradicting DocxDiffSettings.Deterministic. Filed as #621; the harness folds those names before digesting so real changes are not buried under naming churn.

One bug this PR's own review caught

Skipping the attribute sort for single-attribute elements was briefly wrong: canonicalization strips wp:docPr/@id by consulting attribute.Parent, so deciding an attribute's fate after detaching it changes the verdict. The LINQ chain it replaced got this right by accident (ToList() forces evaluation before RemoveAttributes()). Three tests now pin that path; the first fails against the wrong ordering.

The concurrency needed a guard, and it is not an optimization

wasm/DocxodusWasm/DocxodusWasm.csproj does not set WasmEnableThreads, so the browser runtime is single-threaded: Task.Run queues the delegate for the one thread that would then block on the result, and the runtime refuses rather than deadlocking —

PlatformNotSupportedException: Cannot wait on monitors on this runtime

Measured, not inferred: with the guard forced open, all ten npm/tests/docx-diff.spec.ts cases fail with exactly that exception; with it in place they pass (10 passed, 14.0s). Both call sites go through Docxodus.Internal.ParallelWork, compiled out under WASM_BUILD and additionally requiring Environment.ProcessorCount > 1. It preserves the sequential failure ordering, and a concurrent failure is always observed rather than left as an unobserved task exception.

The browser keeps most of the win regardless, because the larger half does not depend on threads: roughly 1.4–1.6× sequential against ~2.3× concurrent.

On the 10 comparisons/second question that prompted this

Not reached for a cold pairwise redline of a document this size. At ~350 ms this is just under 3/s; 10/s means 100 ms, and ~45 ms of that is package I/O no design avoids (13 ms to parse every story, 17 ms to re-serialize a package unchanged). The data products are much closer: with both documents already read, the diff itself is about 60 ms.

Three levers remain, filed rather than buried: #617 (reusable snapshot so bulk pipelines read a document once — the one that decides 10/s), #618 (UnidHelper hashes twice per element for elements the IR never reads back), #619 (MarkupCompatibilityNormalizer full-parses document.xml on every call to find nothing). A fourth — spend the remaining two cores — is deliberately not filed; FINDINGS.md says why.

Files

benchmarks/docxdiff-stress/ is a new standalone harness, outside Docxodus.sln so it never touches CI, packaging or the warning baselines. Its README covers the variants, the corpus differential, the rename-invariant digest and the measurement traps (force a collection between cases; warm past tiered JIT before timing — an early draft reported UnidHelper at 108 ms where the steady state is 51 ms). FINDINGS.md carries the full stage attribution.

docs/architecture/ir_diff_engine.md previously described the renderer's re-read as unconditional; it now documents the single-read design and why the fan-out is gated.

🤖 Generated with Claude Code

https://claude.ai/code/session_01YQ7aHQV9e11SSt4idtyWaK

[JSv4]

CI note — the one red check on 2c9a0d5 is not this PR's.

Failing: test (Playwright) → tests/demo-arcade-freedoom.spec.ts:217 › autopilot navigates the real geometry and collects a Freedoom pickup spot. Everything else passed: 659 passed, 1 failed, 10 skipped.

Why it isn't this PR's:

• 2c9a0d5 changes one markdown file (benchmarks/docxdiff-stress/FINDINGS.md, adding links to DocxDiff re-reads the same document on every comparison — offer a reusable snapshot so bulk pipelines pay for a read once #617/UnidHelper hashes twice per element for all 15k elements of a read, and the IR reads almost none of them back #618/MarkupCompatibilityNormalizer full-parses document.xml on every call because the part contains the string "pPr" #619). The identical code passed this same job twice on this branch — 8443e7a in 18m43s and 1abec08 in 27m26s.
• main is green on this workflow, including eaa62c95, the commit that made Freedoom E1M1 the arcade default.
• The diff touches DocxDiff, IrReader, UnidHelper and IrHasher. The failing test navigates a raycaster autopilot around a game map and never reaches a comparison — it times out before the first pickup.

What the failure looks like: the flight recorder dumped frames: 1401 against the test's 240 s budget — under 6 fps — with stuck: 15. That reads as a starved runner, not a navigation change. This is a recurrence of #492 (Flaky: Freedoom autopilot test times out waiting for the first sigil pickup), which is closed; the diagnostic dump quoted above was added by that issue's fix. I've reopened it with this occurrence.

No fix to port — nothing open addresses it, and making the autopilot's time budget robust is an arcade-demo change with no relationship to this diff, so widening this PR to carry it would be the wrong call.

Re-running the failed job once. If it goes green that confirms the diagnosis; if it fails again I'll come back to it rather than leave it.

---

Generated by Claude Code

[JSv4]

Correction to the previous comment: I cannot re-run the job — rerun-failed-jobs returns 403 Resource not accessible by integration. The diagnosis above is unchanged, but the confirming re-run needs someone with write access, or any later push to this branch.

Everything else on 2c9a0d5 is green (659 passed). I'm keeping the PR watched and will report if the state changes.

---

Generated by Claude Code

[JSv4]

Adversarial review

I treated every claim here as unproven and re-derived it. Most of them hold. The fixes for the ones that do not are in #622, which targets this branch so this PR can land complete.

What I independently reproduced

• The corpus evidence. Built the harness against main and against this branch, ran --corpus TestFiles on both: 678 documents → 8,136 digests, all identical. The number and the result are exactly as stated.
• The single-read reasoning. IrProvenance.Equals returns true for any other instance and GetHashCode returns 0; RetainSources feeds nothing but ReadContext.Provenance() and IrDocument.Sources; nothing in IrEditScriptJson serialises provenance. Handing the renderer the script-side snapshot is sound.
• The concurrency purity claim. Every static reachable from IrReader.Read is an initialize-once read-only collection; the only mutable statics in the library sit in WmlComparer (not on this path) and FontFamilyHelper (a ConcurrentDictionary and a Lazy). The reads really are independent.
• The WASM guard's premise. DocxodusWasm.csproj references the library with AdditionalProperties="WASM_BUILD=true", so ParallelWork.CanFanOut compiles to a constant false; WasmEnableThreads appears nowhere in the repository. The library builds clean under -p:WASM_BUILD=true.

The one behaviour regression

GetRevisions' identical-bytes shortcut is not "the same guard ToRedline uses". ToRedline's identical-bytes path deliberately still runs the compatibility pre-flight. The new one does not.

// passes on main, fails on this branch
var doc = /* a document containing an oMath run */;
DocxDiff.GetRevisions(doc, new WmlDocument(doc),
    new DocxDiffSettings { ThrowOnCompatibilityWarning = true });   // throws on main; returns [] here

Same for OnCompatibilityWarning, which stops firing. DocxDiffComparison's own remarks promise the opposite: "Compatibility preflight … therefore fires on the first product call, exactly as it fires inside each stateless static." The pre-flight is a property of the inputs, not of whether they differ.

Why the validation could not have caught it

This is the part worth taking from the review. The defect has no output-digest signature — a product that stops warning still returns the right answer — so 4,161 tests and 8,136 output digests were both structurally incapable of seeing it. Two coverage gaps follow:

1. Corpus mode never engages the pre-flight. OnCompatibilityWarning/ThrowOnCompatibilityWarning are never set in Corpus.Record, so a second observable output of every product goes undigested.
2. Corpus mode never calls Consolidate. This PR restructured the N-way path — ReadReviewerSet, the IrCompositeMarkupRenderer hand-off, ParallelWork.Fan — and none of the three pairwise products touches any of it. The 678 documents → 8,136 digests headline covers zero consolidate calls; N-way parity rests entirely on the uncommitted reference document.

#622 closes both. The differential becomes 678 documents → 14,916 digests, still green.

Smaller findings

• IrReader.Read leaks an open package on the error path. The OpenXmlMemoryStreamDocument is constructed and GetWordprocessingDocument() called before the try, so an .xlsx handed to the reader (GetDocumentType detects exactly that and throws) leaks what the previous using disposed.
• ParallelWork.Fan's doc comment overstates the success path. The collect loop rethrows at the first faulted task, leaving later faulted tasks unobserved — the very thing the head-failure path takes care to avoid.
• IrHash.ComputeUtf8 allocates 3× on large subtrees. GetMaxByteCount is 3n+3 and ArrayPool<byte>.Shared stops pooling above 1 MB, so for a large canonical subtree it allocates three times the string's UTF-8 length and drops it — worse than the exact-sized array it replaced.
• Two stale documents. DocxDiffComparison's remarks still say the diff read does not retain sources (it does now, and the snapshots pin the parsed XML of every story for the instance's lifetime — a real retention increase worth stating). docs/architecture/ir_diff_engine.md is updated by this PR but the class's own XML docs were not.
• The documented reproduction does not build. dotnet run … --corpus TestFiles --baseline main.json # on main fails: main has no InternalsVisibleTo Include="DocxDiffStress", so the harness dies with a wall of CS0122 that reads like a harness bug. I hit this producing the baseline above; adding that one line to the checkout being baselined is the fix, and the README should say so.
• The warning count did move. Measured with --no-incremental on both branches: library 131 → 132, test project 774 → 775. Both from Docxodus/Internal/ParallelWork.cs — SA1633 fires once per file because no library file carries a StyleCop header, so any new .cs moves both by one. Not a defect; CLAUDE.md's stated baselines just need to move with it.
• CHANGELOG says "all 36 digests", which was true of the eight-variant run but not of the corpus evidence this PR ended up resting on.

Two observations, not defects

• The renderers' re-read fallback is exercised only by tests. Both Render overloads have one production call site each and both supply the snapshots, so the optional parameters are never null in the shipped library while the unit tests call the 4-argument form throughout. Worth knowing before deleting either path.
• UnidHelper.ContentSignature's new walks recurse over subtree depth where element.Descendants() did not. AssignDescendantsDeterministic already recursed, so this is not a new class of failure — it roughly doubles the frame count at a given depth.

On the ContentSignature and CleanAttributes rewrites

I checked these character by character against the originals, since they are the changes most likely to silently move a hash. Both are faithful: the fused walk appends the same characters in the same order ((string)XElement is Value, Descendants and the new recursion agree on document order, the block-container bail-out reaches the same verdict), and the single-attribute path decides while the attribute is still attached, which is what the ToList() in the LINQ chain was doing by accident. The per-call signature cache is keyed on the exact hashed string, and a container's tag-name key can never collide with a non-container's input because the latter always contains |.

---

Generated by Claude Code