perf(semantic): index re-exports at build time instead of scanning all exports per lookup - #88
Conversation
…l exports per lookup Problem ------- `ReferenceFinder::find_refs_recursive` looked for barrel files re-exporting the current file by iterating the exports of EVERY file in the workspace and resolving each re-export specifier, on every visited (file, symbol) node. That made cross-file reference finding O(visited_nodes x total_exports_in_workspace), which dominates runtime on large monorepos with barrel-file chains. Solution -------- `WorkspaceAnalyzer` now builds a reverse re-export index once at construction time (`build_reexport_index`, called next to `build_import_index`): `resolved_source_file -> [(reexporting_file, export)]`. The traversal is a single FxHashMap lookup instead of a full scan. Key changes ----------- - analyzer: add `reexport_index` (FxHashMap) + `build_reexport_index`, plus a shared `resolve_workspace_specifier` helper so the re-export index resolves specifiers exactly like the import index does (same resolver options, same `is_workspace_specifier` guard, same `simple_resolve_relative` fallback, same strip-to-cwd-relative normalization). `build_import_index` is untouched. - reference_finder: replace the workspace-wide scan with an index lookup, keyed through the extracted `normalize_path` helper that `paths_equal` now also uses, so lookup keys keep the exact previous matching semantics (cwd-relative, no case folding, no canonicalization). - profiler: the previously dead `reexport_checks`/`reexport_time_ns` counters now measure the (cheap) index lookup, so `--profile` output stays meaningful. Testing ------- - New integration tests (per-test TempDir, no shared fixture) written first and verified green against the old implementation: named barrel re-export, barrel-of-barrels multi-hop chain, `export * from` wildcard re-export, and a negative case (consumer of a different symbol from the same barrel is not affected). - New unit tests for index construction: named/aliased/wildcard re-exports, barrel-of-barrels, external and import-only files excluded, and keys matching the workspace-relative paths used by `exports`/`import_index`. - Synthetic 3.7k-file workspace with barrel chains: 3.36s -> 0.19s wall (~17x), identical affected output; `--profile` shows resolution calls dropping from 25,970,412 (~2.0s) to 0 with symbol lookups unchanged (7,212).
The new barrel-test helper omitted 'git branch -M main' (all 13 pre-existing git-init sites in this file have it), so the 4 new tests failed on any machine with init.defaultBranch != main. Verified under defaultBranch=master. Also document the new reexport_index in CLAUDE.md's Critical Data Structures.
|
Warning Review limit reached
Next review available in: 42 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
📦 Preview Release AvailableA preview release has been published for commit 146f9a6. Installationnpm install https://github.com/frontops-dev/domino/releases/download/pr-88-146f9a6/front-ops-domino-2.0.0.tgzRunning the previewnpx https://github.com/frontops-dev/domino/releases/download/pr-88-146f9a6/front-ops-domino-2.0.0.tgz affectedDetails |
The ort merge strategy silently interleaved this branch's test additions with sibling PRs' tail-appended tests, without leaving conflict markers: - src/semantic/analyzer.rs: test_build_reexport_index_keys_match_analyzer_paths lost its closing braces, merging into the following #90 test. - tests/integration_test.rs: this branch's scaffold_repo/barrel-test tail section was interleaved with #86's Turborepo test-helper tail section. Restored both sections to their independent, complete bodies, ran cargo fmt, and dropped the now-removed TrueAffectedConfig.root_ts_config/ include/ignored_paths fields (#83) from the struct literals that still referenced them.
The automatic merge of origin/main interleaved this branch's appended barrel/re-export tests with main's appended tests, truncating test_barrel_consumer_of_other_symbol_not_affected mid-assertion and leaving unclosed delimiters. Reconstructed as main's file plus this branch's self-contained barrel block. Verified nothing was lost: all 4 barrel tests and main's turbo/mts/relative-roots tests are present and passing (integration 83, lib 244, cli 16).
2 participants
Problem
The hottest loop in reference finding: every
find_refs_recursivecall scanned all exports of all workspace files to find barrel files re-exporting the current file — O(visited_nodes × total_exports_in_workspace). On barrel-heavy monorepos this dominates the run (a 3.7k-file synthetic showed ~26M resolution calls for one changed symbol). Found in the full repo review (perf finding #6).Solution
A reverse re-export index built once at analyzer construction —
reexport_index: FxHashMap<PathBuf, Vec<(PathBuf, Export)>>mapping resolved source file → re-exporters — exactly mirroring howimport_indexalready made the forward direction O(1). The reference-finder loop becomes a single map lookup.Key changes
src/semantic/analyzer.rs:build_reexport_index(cwd)called right afterbuild_import_index(which is byte-for-byte untouched — merges cleanly with the parallel-import-index PR, verified viagit merge-tree); sharedresolve_workspace_specifierhelper replicates the exact query-time resolution chain (is_workspace_specifierguard →resolver.resolve→strip_prefix(cwd)→simple_resolve_relativefallback) so build-time and on-demand resolution cannot divergesrc/semantic/reference_finder.rs: the workspace scan replaced by the lookup;normalize_pathextracted andpaths_equaldelegates to it, so index keys and comparisons share one normalization by constructionsrc/profiler.rs: the previously-deadrecord_reexport_checkcounter is now live and measures the lookupPerformance (measured, release builds)
--profilecorroborates the mechanism: query-time resolution calls 25.9M → 0 (reviewer's independent bench: 37,449 → 9), symbol lookups identical (traversal provably unchanged), identical affected output.Testing
Characterization-first: 4 new integration tests (named barrel re-export, barrel-of-barrels multi-hop,
export *, and a negative case asserting a consumer of a different symbol from the same barrel is NOT affected) were run green against the unmodified code first, then shown to genuinely gate the new path (all 4 fail with the index stubbed empty — reviewer-verified). 2 unit tests pin index construction and key shape.Second-model review did differential testing against
mainon 1k-file synthetics (identical affected sets, aliasing/wildcard edge cases included) and verified resolution-chain equivalence step by step. Its one required fix (the new test helper missedgit branch -M main, breaking oninit.defaultBranch != mainmachines) is in the second commit, verified underdefaultBranch=master.Tallies (
--no-default-features, fixture populated): lib 215; integration 65/67 (failures = pre-existingtest_three_dot_diff_behavior+ the pre-existingtest_jsx_to_tsx_extension_resolutionflake, which passes 3/3 in isolation; both broken on unmodified main); cli 15; clippy/fmt clean. CLAUDE.md's Critical Data Structures section documents the new index.Part of the repo-review fix wave (#83, #84).