Follow-up from the comprehensive review of PR #239 (LSP server). Line numbers reference the PR branch at review time.
Severity: major (1-2), minor (3-6) — per-keystroke cost has no shedding, and caches grow without bound. Items 1-2 were measured with temporary release-mode probes during review (probes deleted); their adversarial verification pass was cut short by infra limits, so treat numbers as reported-by-one-agent, but the code paths are straightforward to confirm.
- Every didChange synchronously re-runs the full pipeline (closure reparse + full type check + all analysis rules) inside the single-threaded message loop (
server.rs:214). capabilities.rs pins TextDocumentSyncKind::FULL, VS Code sends one didChange per keystroke, messages are handled strictly in order, and $/cancelRequest is dropped — so a burst of 10 keystrokes queues 10 back-to-back full analyses and the hover/completion request behind them waits for all of it. Measured single-file recompute: 0.71 ms (5.8 KB), 3.97 ms (59 KB), 12.5 ms (240 KB) — release mode, no imports; cost scales with the whole import closure, and debug builds are ~an order of magnitude slower. Cheap fix: drain consecutive queued didChanges for the same document and analyze only the final text; longer term, defer diagnostics to a short idle timer and honor $/cancelRequest.
publishes_with_dependents multiplies that by the number of open dependents (server.rs:145): a keystroke in a shared imported file forces every open dependent through its own full-closure pipeline inside the same notification turn (per-entry analyses share nothing). Measured: 3.7 ms/keystroke with 1 open dependent → 9.7 ms with 4 → 17.8 ms with 8 (30 KB shared lib); the same eager path also fires on every didOpen while any memoized analysis has an unresolved import (database.rs:110 widening). Fix: publish eagerly only for the changed document; republish dependents lazily or from a debounced idle pass.- Synchronous disk I/O per recompute (
loader.rs:40): every recompute re-reads every unopened closure file (read_to_string), calls canonicalize per dependency (project.rs:386, project.rs:568), and — while the user is mid-typing a use line — runs a read_dir + edit-distance sibling scan per missing import, per keystroke.
- ~3 full copies of every closure file's text per open document (
analysis.rs:265): arena SourceFileData.source + ClosureFile.source (fresh Arc<str>) + LineIndex.text (owned Box<str>), and K open documents sharing a closure duplicate all of it K times.
Analysis::line_index clones the whole document text per request (ide/ide/src/lib.rs:146) — O(doc) alloc+memcpy per request, once per open doc per publish cycle, plus an extra clone per same-file goto target.- No eviction anywhere:
RootDatabase::analysis memoizes a FileAnalysis for ANY requested path — including URIs never opened (feature requests trigger disk reads via the VfsLoader::read fallback, handlers.rs:23) — and nothing is ever evicted in a long editor session.
Follow-up from the comprehensive review of PR #239 (LSP server). Line numbers reference the PR branch at review time.
Severity: major (1-2), minor (3-6) — per-keystroke cost has no shedding, and caches grow without bound. Items 1-2 were measured with temporary release-mode probes during review (probes deleted); their adversarial verification pass was cut short by infra limits, so treat numbers as reported-by-one-agent, but the code paths are straightforward to confirm.
server.rs:214).capabilities.rspinsTextDocumentSyncKind::FULL, VS Code sends one didChange per keystroke, messages are handled strictly in order, and$/cancelRequestis dropped — so a burst of 10 keystrokes queues 10 back-to-back full analyses and the hover/completion request behind them waits for all of it. Measured single-file recompute: 0.71 ms (5.8 KB), 3.97 ms (59 KB), 12.5 ms (240 KB) — release mode, no imports; cost scales with the whole import closure, and debug builds are ~an order of magnitude slower. Cheap fix: drain consecutive queued didChanges for the same document and analyze only the final text; longer term, defer diagnostics to a short idle timer and honor$/cancelRequest.publishes_with_dependentsmultiplies that by the number of open dependents (server.rs:145): a keystroke in a shared imported file forces every open dependent through its own full-closure pipeline inside the same notification turn (per-entry analyses share nothing). Measured: 3.7 ms/keystroke with 1 open dependent → 9.7 ms with 4 → 17.8 ms with 8 (30 KB shared lib); the same eager path also fires on every didOpen while any memoized analysis has an unresolved import (database.rs:110widening). Fix: publish eagerly only for the changed document; republish dependents lazily or from a debounced idle pass.loader.rs:40): every recompute re-reads every unopened closure file (read_to_string), callscanonicalizeper dependency (project.rs:386,project.rs:568), and — while the user is mid-typing auseline — runs aread_dir+ edit-distance sibling scan per missing import, per keystroke.analysis.rs:265): arenaSourceFileData.source+ClosureFile.source(freshArc<str>) +LineIndex.text(ownedBox<str>), and K open documents sharing a closure duplicate all of it K times.Analysis::line_indexclones the whole document text per request (ide/ide/src/lib.rs:146) — O(doc) alloc+memcpy per request, once per open doc per publish cycle, plus an extra clone per same-file goto target.RootDatabase::analysismemoizes aFileAnalysisfor ANY requested path — including URIs never opened (feature requests trigger disk reads via theVfsLoader::readfallback,handlers.rs:23) — and nothing is ever evicted in a long editor session.