perf: single-pass Descendants traversal in GetAnchors - #3839
Open
Mpdreamz wants to merge 3 commits into
Open
Conversation
GetAnchors previously called document.Descendants<T>() seven separate times (IncludeBlock, HeadingBlock, DirectiveBlock × 4 with different OfType filters, InlineAnchor), each a full recursive walk, plus materialised the results seven times. The profiler measured ~24s in Stack.PushWithResize / ToArray / ToList across these traversals. The deeper problem was GetPrecedingHeadingLevel: called once per IncludeBlock, it called root.Descendants().ToList() followed by allBlocks.IndexOf(block) — O(n) traversal + O(n) linear scan — making the whole GetAnchors call O(n·k) for k includes per document. Fix: - A single document.Descendants() pass collects typed lists (headings, directives, inlineAnchors) and a flat ordered list with a position dictionary in one O(n) sweep. - All subsequent filter/select operations work over the typed lists instead of re-traversing. - GetPrecedingHeadingLevel now accepts the prebuilt list and position index: O(1) lookup + O(backward scan) instead of O(n) re-traversal per call. Added MultipleIncludesInterleavedWithHeadingsTests to guard the position-index correctness: two stepper includes after separate h2 headings must each get the right preceding heading context. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The previous approach built a full List<MarkdownObject> (allNodes) and a Dictionary<MarkdownObject, int> (nodePositions) covering every block in every document so that GetPrecedingHeadingLevel could do an O(1) lookup. Both collections were allocated on every file even though GetPrecedingHeadingLevel is only ever called on files that have found IncludeBlocks — a small minority. A simpler approach: since the DFS pass already visits nodes in document order, the preceding heading level for an IncludeBlock is simply the last HeadingBlock seen before it in the traversal. Track lastHeadingLevel as a plain int? and annotate each IncludeBlock with it at discovery time. This eliminates allNodes, nodePositions, and GetPrecedingHeadingLevel entirely. Files without IncludeBlocks pay only for 3 typed lists and a single traversal — no dictionary overhead. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
GetAnchorspreviously calleddocument.Descendants<T>()7 times (IncludeBlock, HeadingBlock,DirectiveBlock× 4 with differentOfTypefilters, InlineAnchor), each a full recursive walk, plus materialised the results 7 times.The deeper problem was
GetPrecedingHeadingLevel: called once perIncludeBlock, it calledroot.Descendants().ToList()followed byallBlocks.IndexOf(block)— O(n) traversal + O(n) linear scan per call — making the wholeGetAnchorscall O(n·k) for k includes per document.Fix: a single
document.Descendants()pass that collects all typed lists at once. During the pass alastHeadingLevelvariable is updated whenever aHeadingBlockis visited; eachIncludeBlockis immediately annotated with the current value. This means the preceding heading level is available for free with no extra allocation —GetPrecedingHeadingLevelis deleted entirely.Files without include blocks (the majority) now pay only for 3 small typed lists and a single O(n) traversal.
IncludeBlock/StepBlock/ChangelogBlock/SettingsBlockall extendDirectiveBlock, so one switch case covers them all.Test plan
dotnet test tests/Elastic.Markdown.Tests/ --filter HeadingOrderTests|InlineAnchorTests|AnchorLinkTests— 17 tests passdotnet test tests/Elastic.Markdown.Tests/— full 1954-test suite passes (includesMultipleIncludesInterleavedWithHeadingsTestswhich guards the preceding-heading annotation)dotnet run --project src/tooling/docs-migrate -- bench— recordResolveDirectoryTreetime🤖 Generated with Claude Code