perf(toc): avoid repeated dependency traversal when filtering entries - #2285
stenin-nikita wants to merge 1 commit into
Conversation
|
There was a problem hiding this comment.
One point about where the time actually goes. After this PR ~99% of the remaining getter time is overallOrder() itself: in circular mode dependency-graph finishes with keys.filter(node => result.indexOf(node) === -1), which is O(N^2).
Measured on 22k-node graphs (1x22000, 200x110, 50x440 TOC/entries):
overallOrder(): 1037 / 937 / 1078 ms- same traversal with a Set instead of
indexOf: 7 / 9 / 6 ms, identical order dependantsOfover all 22k entries: 7-8 ms
A Set-based overallOrder override in src/core/utils/graph/index.ts gives another ~100x on top of this PR and also speeds up tocs, init() and serialize() (which runs once per worker). What do you think about this?
| for (const tocPath of allTocPaths) { | ||
| const isRootToc = tocPath.split('/').length === minNestingLevel; | ||
|
|
||
| if (isRootToc || this.relations.dependenciesOf(tocPath).length > 0) { |
There was a problem hiding this comment.
Before we optimize this check, is it worth asking whether it ever changes the result?
The set is built from TOCs that are either root or have at least one dependency. But the only TOCs we ever look up in it are the ones that reach the entry through the graph, and every such TOC has dependencies by definition. So for every lookup the answer is "yes" whenever the node is a TOC at all. The root / nesting-level part never gets to decide anything, and the existing test "should exclude entries from unreferenced nested TOC files" quietly confirms this: nothing is excluded.
If that reading is right, the getter is really just "keep entries that some TOC owns", and the whole eligibility block can go instead of being made faster. That also removes the per-TOC traversal entirely, which is what this PR is after.
One thing to keep in mind if you go that way: the original intent from #1625 (keep only root TOCs and TOCs referenced by includes) should not be restored either. Outside watch mode included TOCs are registered as plain sources and no TOC references another, so restoring the intent would drop every page from any TOC deeper than the root one.
2 participants
Description
For large documentation projects,
TocService.entriesrepeatedly traversed the same TOC dependencies for every page. Resolve eligible TOCs once per getter call and use a set during page filtering, preserving entry order and reflecting subsequent graph changes.Add regression tests for dependency traversal work, graph mutations between reads, and shared entries with cyclic TOC references.
Validation
npm test: 2186 passed, 1 skipped.npm run typecheckand ESLint for the changed files passed.