Skip to content

Make dead_code and outline cost follow the scoped repo, not the whole store #1002

Closed

Description

@zzet

Why

analyze kind=dead_code and explore operation=outline (get_repo_outline) take time in proportion to the whole store, not to the repo the session is bound to. The tools benchmark runs every tool against a store holding only gortex, then against a store holding six repos (n=15 per tool):

  • dead_code p50: 6.0 s on the gortex-only store, 17.4 s on the six-repo store. Tiny repos in the six-repo store take 17-19 s too.
  • outline p50: 4.3 s on the gortex-only store, 12.6 s on the six-repo store (12.6-13.7 s for every repo).

Both are slow-lane tools. One dead_code call or two outline calls keep the slow lane full for 13-19 s, and other slow calls queue behind them. Adding a repo to the store should not make tools slower on a repo that has nothing to do with it.

Current state (main)

dead_code

  • internal/mcp/tools_enhancements.go handleFindDeadCode calls analysis.FindDeadCode(..., s.getProcesses(), nil, opts) over the whole store. Only afterwards does it filter rows to the session and repo allow-set. That filter calls reader.GetNode(e.ID) once per entry through analyzeNodeVisible, so it makes one store read per row.
  • internal/analysis/deadcode.go FindDeadCode runs three whole-store scans:
    • buildIfaceRequiredMethods, which calls IfaceImplementsRows() and reads every implements edge.
    • collectDeadCodeCandidates, which calls DeadCodeCandidates and reads every candidate node.
    • g.NodesByKind(graph.KindFile), which reads every file node to find entry-point files.
  • FindDeadCodeOptions has no repo field, only SkipCrossRepoNodes. DeadCodeEntry carries only ID, name, kind, file path and line.
  • The store interfaces DeadCodeCandidator and IfaceImplementsScanner in internal/graph/store.go, and their SQLite versions DeadCodeCandidatesContext and IfaceImplementsRows in internal/graph/store_sqlite/store_analysis.go, take no scope argument. Since commit 4408bb2 these reads are paged and bound to the request context, but they still cover the whole store.

outline

  • internal/mcp/tools_outline.go handleGetRepoOutline does the following for a bound session:
    • It loads every scoped node (scopedNodesLight) into an inScope ID map, and computes total nodes and the language breakdown from that list.
    • It walks reader.AllEdges() only to count the edges that have both endpoints in scope.
    • It filters the global getCommunities() and getHotspots() results by membership in inScope.
  • mostImportedFiles passes every scoped node ID to FileImportCounts(scope), and falls back to an AllEdges() loop on backends that lack that call. entryPoints runs a global FindNodesByName("main") and then filters it with inScope.
  • Per-repo node and edge aggregates already exist and can be reused: Store.RepoStats() in internal/graph/store_sqlite/store.go and Graph.RepoStats() in internal/graph/graph.go.

v0.64.11 is planned to replace the outline's AllEdges() edge count with a SQL count by repo prefix. That change aims for outline ≤ 1 s and leaves the rest of the outline as it is. This issue builds on it.

What to do

dead_code

  • Add RepoPrefixes []string to FindDeadCodeOptions. In handleFindDeadCode, fill it from the resolved repo allow-set and the session scope.
  • Add scoped store variants:
    • DeadCodeCandidatesContext(ctx, kinds, inEdgeKinds, repoPrefixes), which filters with repo_prefix IN (...) on the candidate side only. Incoming edges from any repo still count as usage, so a symbol whose only callers are in another repo stays alive.
    • IfaceImplementsRows(repoPrefixes), which returns only implementing types in scope but keeps interfaces from any repo.
    • NodesByKindInRepos, used for entry-point files.
    • An in-memory reference implementation of each, plus coverage in the storetest conformance suite.
  • Keep only the processes whose entry point is in scope.
  • Remove the one-GetNode-per-row post-filter. Carry the repo prefix and workspace on DeadCodeEntry internally, as fields that are not serialized, so the JSON output does not change.

outline

  • On the bound-session path:
    • Take total nodes, the per-language breakdown and the edge count from per-repo aggregates instead of scopedNodesLight. Use RepoStats, or add a StatsForRepos(prefixes) that also counts edges with both endpoints in the scoped repos.
    • Add FileImportCountsForRepos(prefixes): a GROUP BY over import edges joined to the target node's repo prefix. It replaces the call that passes every node ID.
    • Find entry points with a repo-scoped query (FindNodesByNameInRepos or equivalent).
    • Use per-repo community and hotspot results when they are cached. Otherwise compute hotspots for the scoped repos only.
  • Cache the outline response per (repo-prefix set, graph revision), so repeat calls cost O(1) until the next mutation.
  • If you add per-repo counters, they must stay exact across untrack, re-track and retirement. Until a counter is marked ready, fall back to COUNT.

Out of scope

  • Making the remaining tools independent of store size (call_chain, explore.context, callers, search and the other AllEdges() call sites in internal/mcp). That work is tracked separately.
  • Slow-lane and dispatch changes.
  • The empty hotspot results that hotspot consumers return before the first analysis pass.
  • Changes to the dead-code heuristics.

Done when

  • dead_code p50 for each repo in the six-repo store is ≤ 1.2x the p50 of the same repo on a store holding only that repo. That means gortex ≤ 7.2 s and small repos ≤ 1 s, measured with the tools benchmark (n=15) on the reference host. Current candidate results: all six ratio-of-p50 checks pass; both estimators and the corpus-2 paired-ratio qualification are retained.
  • TestDeadCodeScopedRowsIndependentOfOtherRepos exists and passes. It counts the rows returned by the store reads behind a scoped dead_code call (candidates, implements rows and file nodes) and checks that the count for one repo changes by ≤ 5% when a larger unrelated repo is added to the fixture store.
  • TestDeadCodeScopedMatchesPostFiltered exists and passes: on a multi-repo fixture, the scoped result equals the old result (whole-store scan, then filter).
  • TestDeadCodeCrossRepoCallerKeepsSymbolAlive exists and passes: a symbol whose only caller is in another repo is not reported as dead.
  • Uncached outline p50 is ≤ 300 ms for every repo on the six-repo store. A repeat call with no mutation in between is ≤ 50 ms. Current candidate n=15 results: 90 first calls and 90 repeats pass; fresh response cache per daemon start and ordinary readiness before measurement.
  • TestOutlineBoundSessionNeverScansAllEdges exists and passes: a counting reader records zero AllEdges and zero AllNodes calls on the bound-session path.
  • TestOutlineGoldenSixRepo exists and passes: for the same scope, the output is identical to the current implementation.
  • If per-repo counters are added, TestRepoEdgeCounterExactAfterRetrack exists and passes: counts equal a fresh COUNT after untrack, re-track and retirement. Not applicable: this implementation adds no incrementally maintained per-repo counters. It uses exact SQL aggregates and revision-bound prepared summaries; a mutation invalidates the summary and restores the exact query path.
  • No regression: unbound-session outline output is byte-identical to the current output, and on a single-repo store the p50 of both tools is ≤ 1.10x the v0.64.10 p50. Final candidate n=15 results and actual-edit cache check: all 45 groups / 135 calls succeeded; all 15 unbound candidate/main bodies match. Bound candidate/tag p50 ratios are 0.000247 (outline) and 0.323155 (dead code).
  • go test -race ./internal/analysis/... ./internal/mcp/... ./internal/graph/... passes. CI on current candidate 18c027826 is green: 25 successful checks and one expected skip. The test-selection artifact includes all seven required packages in the Linux race jobs (96 affected packages per OS), with the prescribed SQLite/libc race flags. Local validation also passed 180 affected race tests and the full SQLite suite (3,265 passing records, zero failures, seven documented skips).

How to verify

  • Unit and golden tests: go test -race -run 'DeadCode|Outline|RepoEdgeCounter' ./internal/analysis/... ./internal/mcp/... ./internal/graph/...
  • Add BenchmarkDeadCodeScopedSixRepo, then run go test -bench DeadCodeScoped -run ^$ ./internal/analysis/....
  • Tools benchmark: on the same host, run the per-tool sweep (n=15) against a gortex-only store and then against the six-repo store. Compare dead_code and outline p50 for each repo with the targets above, and with v0.64.10 on the single-repo store.
  • Outline cache: call outline twice on the same repo with no edit in between, then once after an edit. The second call should hit the cache and the third should not.

Dependencies

Blocked by (must land first):

Related issues

Planning

  • Milestone: v0.64.12
  • Priority: latency (first priority)
  • Estimate: ~5 engineer-days

Activity

  1. added this to the v0.64.12 milestone on Oct 25, 2026
  2. added
    mcpMCP tool surface, schemas, facade and errors
    analysisanalyze kinds, change.* and derived rollups
    performanceLatency, memory and disk cost
    p1: latencyResponsiveness: tool-call, edit-loop, concurrency, restart/stop timing
    on Oct 25, 2026
  3. moved this to Todo in Gortexon Oct 25, 2026
  4. zzet commented on Oct 25, 2026

    @zzet
    OwnerAuthor

    Acceptance update for v0.64.11: the current structural improvement in #994 is accepted, and its unmet 1-second steady-state outline target on the six-repository store is explicitly deferred to this issue.

    The existing stronger goals here—uncached outline p50 ≤300 ms and unchanged repeat calls ≤50 ms—remain in place. The current implementation has no per-scope outline response cache or new persistent counters. Measured limitations and diagnostic results are recorded at #994 (comment).

    Implementation remains scheduled for v0.64.12, after acceptance of the current milestone.

  5. moved this from Todo to In Progress in Gortexon Oct 27, 2026
  6. zzet commented on Oct 29, 2026

    @zzet
    OwnerAuthor

    PR #1147 has five named functional acceptance checks already passing at published revision a6b0cf2d5664decc705009169c4b1b797a74d73d:

    • TestDeadCodeScopedRowsIndependentOfOtherRepos
    • TestDeadCodeScopedMatchesPostFiltered
    • TestDeadCodeCrossRepoCallerKeepsSymbolAlive
    • TestOutlineBoundSessionNeverScansAllEdges
    • TestOutlineGoldenSixRepo

    The retained affected race run passed 133 top-level executions / 592 including subtests, with zero failures or skips. It also covered memory/SQLite conformance, generation/scope isolation, cache invalidation, output parity and retained-result ownership. Pinned formatting/lint/build checks passed before that push; its hosted CI was green. These results remain attributed to that revision, not to the integration now being edited.

    The PR is being integrated with current main c96cd123aefa0fdc5807f0b40bd439d80faccfc9. The actual conflict is between the new outline preparation and main's cancellation-aware analysis: preparation must preserve the analysis context and must not publish a cancelled partial ranking. That integration will receive its affected tests and prescribed checks before the next push.

    No performance gate is accepted yet. The next measurement is the already-planned first retained round comparing candidate, current main and v0.64.10 on independent stores, with rotating matched requests. Its measured cost will determine the remaining n=15 schedule. #1002 remains open and In Progress; all latency/no-regression and full integrated race-suite criteria remain outstanding.

  7. zzet commented on Oct 29, 2026

    @zzet
    OwnerAuthor

    PR #1147 now contains current-main integration at d57aff4253cd1317ba903b9e17b067d075f9b9aa, including cancellation of actual outline preparation work. Affected race batches passed 1,188 events with seven documented unrelated/unsupported skips; all required #1002 controls ran. The final test-only lint correction passed nine repeated controls, and all prescribed pinned gates passed before push. The PR description contains the validation details; current-head CI is pending.

    The first retained native comparison round started at 2026-10-29 23:06:16 UTC: candidate, current main c96cd123 and v0.64.10, using separate owned stores and rotating matched requests. A prior setup invocation refused stale private-driver revision pins before starting any product; that failure is retained and the pins were corrected and tested before this run.

    This first standalone round will establish actual setup/warmup/query cost before scheduling the remaining n=15 matrix. Native observations do not satisfy the separate 8-CPU/16-GiB reference-profile gates. All production performance criteria remain pending; #1002 stays In Progress.

  8. zzet commented on Oct 29, 2026

    @zzet
    OwnerAuthor

    The first matched native diagnostic round is retained for candidate d57aff4, main c96cd12 and v0.64.10. It used the same gortex source revision with separate owned stores and back-to-back requests. This is one sample per call, not the n=15 result or acceptance on the 8-CPU / 16-GB reference profile.

    Call Candidate Main v0.64.10
    dead_code 5,125.930 ms 5,220.767 ms 855.274 ms
    First outline 18.462 ms 2,416.025 ms 6,261.848 ms
    Repeat outline 7.026 ms 2,385.519 ms 6,260.348 ms

    Main and candidate dead_code response bodies match exactly, with 18,030 total entries. The tag reports 203 total entries; all responses use limit 200. That result-population difference must be explained before treating the timing difference as a comparable regression measurement. Main/candidate outline responses differ in community count (831 versus 834) and one edge; repeated responses within each build are stable. Cross-build parity is not accepted against these differing analysis states.

    The complete first round cost 380.94 seconds, including readiness and analysis preparation; the measured request window was 28.55 seconds. All owned processes stopped normally, with no forced termination. Fourteen more cold rounds would cost about 89 minutes for this topology alone. Before expanding the matrix, the next step is a bounded CPU profile of the real dead_code request and inspection of the retained graph/analysis identities. Original evidence and stores remain preserved.

    Local integration checks passed: 1,188 test pass events, with seven classified legacy/unsupported-backend skips, and pinned lint/format/vet checks. Hosted CI on this head has one failing macOS test, TestResourceAndPromptHandlersJoinTheActivityBracket (activity count 1 after each handler); the other test partitions and lint passed. The activity ownership failure is being investigated rather than blindly rerun. The PR remains draft and this issue remains In Progress.

  9. zzet commented on Oct 30, 2026

    @zzet
    OwnerAuthor

    The native comparison exposed an existing fallback correctness bug, now fixed in 1bd7c4b, as well as the slow-path dispatch problem.

    On fresh main/candidate copies of the same retained main database and WAL, the initial, ready and after-call graph/analysis state matched: 191,575 nodes, 1,159,861 edges and the same published analysis generation. The candidate request took 474.519 ms, versus main 5,167.620 ms and the tag 809.127 ms. That diagnostic correctly failed its exact-response assertion: candidate returned 197 entries and main 18,030. The failure remains retained and is not relabeled as a passing benchmark.

    The difference is concrete: 196 of the first 200 main entries were intra-function local bindings. The legacy scan used a negative kind list that omitted Local; the indexed query followed the existing seven-kind candidate list. A small graph with a local binding and real reads/returns_to edges reproduced the disagreement. The fix removes the duplicate negative list and uses the same positive candidate eligibility for both readers. Existing field/variable/constant options and usage heuristics remain unchanged; unknown structural kinds cannot silently become candidates.

    Validation then compared the complete result arrays on a fresh owned copy of the real retained database, using the actual published analysis/process snapshot: all 197 indexed results equal all 197 forced-legacy results, and every public-wire field matches all 197 recorded native candidate entries. This was a race-enabled in-package semantic check, not a new native latency measurement. The small regression failed before the fix and passed afterward. Focused tests and exact pinned lint/format/vet/module/workflow checks passed before the normal push.

    The previously measured indexed path is preserved, but the final 1bd7c4b artifact still needs its native/reference measurements. N=15, six-repo/small-repo isolation, same-state unbound outline parity and reference-profile acceptance remain pending. Hosted CI for the final head is also pending; the PR stays draft and this issue stays In Progress.

  10. 19 remaining items

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    analysisanalyze kinds, change.* and derived rollupsmcpMCP tool surface, schemas, facade and errorsp1: latencyResponsiveness: tool-call, edit-loop, concurrency, restart/stop timingperformanceLatency, memory and disk cost

    Projects

    • Status
      Done

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions