Repository navigation
perf(sync): scan name-matched imports only when exports changed - #592
Merged
Merged
Conversation
Every watcher sync ran reopenNameMatchedImports for each modified file, even when the edit changed no exported name. On n8n's widely imported @n8n/agents/src/index.ts a one-line comment save parsed every importer: `sync-name-matched-imports` took 38-40s, the daemon synced in 45s, and its RSS grew to ~20GB. The extraction now reports exportDelta: the files whose exported-name set changed (adds, drops, renames, re-exports, default moves), read from the bindings table before and after the store. A definition delta alone is not enough — a re-export or a dropped default changes no node — and a non-exported top-level definition changes nothing importers can bind. The sync scans only modules in exportDelta. The same n8n save now finishes in 2.0s with the phase at 0ms. The ~20GB idle RSS that coincides with the deferred whole-graph resynthesis is separate and not addressed here. Full suite: 936 files / 11,197 passed / 44 skipped. The three re-export/default cases in sync-name-matched-imports fail with a definition-only gate and pass with exportDelta.
…reopens importers
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
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.
What changed
exportDelta: the files whose exported-name set changed (adds, drops, renames, re-exports, default moves), read from thebindingstable before and after the store.reopenNameMatchedImports) now runs only for modules inexportDelta. Before, it ran for every modified file, even a comment-only edit.Why
On n8n, editing one line of the widely imported
packages/@n8n/agents/src/index.tsran the scan over every importer:sync-name-matched-importsA definition-name delta is not a sufficient gate: three tests in
sync-name-matched-importsshow a re-export or a dropped default changing what importers bind while no node appears or disappears.exportDeltacaptures those changes from the exported bindings. A non-exported top-level definition (the benchmark's probe marker) changes neither and now skips the scan.Scope / not fixed here
The same save still grows the daemon to ~19–20 GB while the deferred whole-graph resynthesis runs after the sync. That is a separate path (
refreshSynthesis→resynthesizein the daemon's main process); a cold standaloneresynthesizepeaks at 772 MB, so the daemon's warm state is part of it. This PR fixes the scan and the 45 s sync, not that idle footprint. A V8 full GC after the pass did not return it.Tests
sync-name-matched-imports(all cases, including the three re-export/default ones that fail with a definition-only gate),sync-import-retry-crowded-key,exact-match-bare-import-binding,local-import-bindings,sync-default-exports,sync-nested-barrel-route,namespace-reexport: pass.README
No user-visible command, tool, language, framework, target or config change. The pooled-index benchmark doc on the #591 branch is not part of this diff; no README row moves for this change.