From 73096a19c010eec5d41475338a410e3c05bf9f76 Mon Sep 17 00:00:00 2001 From: Atlas from Plumb Date: Tue, 13 Oct 2026 14:16:29 +1100 Subject: [PATCH] fix(topology): read dot-importing files once per census, not per call site The caller census (#648) asked, for every recorded call of a seed's name, whether its file dot-imports a package. The planner answered through the import-kind index, scanning every import node of the workspace per site, so a common method name (Execute: 962 sites, 11k imports) took seconds. On arm R's real tasks a pack took 6-10 s, ran past the 2 s graph deadline and lost its affected-tests section and run-first line. The set is now read once. Replayed in-process on the same index (plumb dfa964fd): R05 7.5-9.6 s -> 0.36 s, R08 6-10 s -> 0.23 s, R03 1.2 s -> 0.25 s; all four Go packs carry the run-first line (R01 control kept). Census claims unchanged. Co-Authored-By: Claude Opus 5.5 Plumb-Session: warm-cobra --- CHANGELOG.md | 9 ++++ internal/topology/callsites_open_test.go | 56 ++++++++++++++++++++++++ internal/topology/callsites_query.go | 33 ++++++++++++-- 3 files changed, 94 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a4f2549a..307a5ac5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -260,6 +260,15 @@ ### Fixed +- **`context_for_task` no longer spends seconds on its caller census, which + starved the affected-tests estimate.** The census (#648) checked, for every + recorded call of a seed's name, whether its file dot-imports a package, and the + database answered that by scanning every import of the workspace each time. For + a common method name (`Execute`, `Close`) a pack took 6–10 s, ran past the 2 s + graph deadline, and lost its affected-tests section, run-first line included. + The dot-importing files are now read once per census: on the real tasks that + starved, a pack takes 0.23–0.36 s and carries its test estimate again. + - **A slow daemon start can no longer produce a second daemon, and waits 30 s, not 10.** When `plumb serve` started the daemon and it took longer than the 10 s wait (a loaded machine, a cold start), serve gave up and released diff --git a/internal/topology/callsites_open_test.go b/internal/topology/callsites_open_test.go index c945f13e..a25835e3 100644 --- a/internal/topology/callsites_open_test.go +++ b/internal/topology/callsites_open_test.go @@ -2,9 +2,11 @@ package topology import ( "context" + "fmt" "path/filepath" "slices" "testing" + "time" ) // TestOpenCallSites pins which calls of a name stay open after a resolution pass: @@ -123,3 +125,57 @@ func TestOpenCallSites(t *testing.T) { t.Errorf("limit 1: listed %d of total %d (err %v), want 1 of 3", len(open), total, err) } } + +// TestOpenCallSites_CostDoesNotScaleWithImportsPerSite pins the shape of the +// census query: whether a call's file dot-imports a package is read once, not +// asked per site. Asked per site, the planner scans every import node for each +// call (PLAN-544: 6–10 s per pack on this repository); here that would be 2,000 +// sites times 20,000 imports. The bound is two orders of magnitude above the +// fixed query's time, so it measures the shape, not the machine. +func TestOpenCallSites_CostDoesNotScaleWithImportsPerSite(t *testing.T) { + db, err := openDB(filepath.Join(t.TempDir(), "cost.db")) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { db.Close() }) + tx, err := db.Begin() + if err != nil { + t.Fatal(err) + } + for f := range 400 { + p := fmt.Sprintf("p%d/f.go", f) + res, err := tx.Exec(`INSERT INTO topology_files(path, language, mtime_ns, content_hash, indexed_at, error_msg) VALUES (?,?,?,?,?,?)`, p, "go", 0, "h", 0, "") + if err != nil { + t.Fatal(err) + } + file, _ := res.LastInsertId() + for i := range 50 { + if _, err := tx.Exec(`INSERT INTO topology_nodes(file_id, kind, name, qualified, signature, start_line, end_line, docstring, language) VALUES (?,?,?,?,?,?,?,?,?)`, + file, string(KindImport), fmt.Sprintf("imp%d", i), fmt.Sprintf("example.com/imp%d", i), "", 1, 1, "", "go"); err != nil { + t.Fatal(err) + } + } + for range 5 { + if _, err := tx.Exec(`INSERT INTO topology_call_sites(file_id, language, site_kind, callee, qualifier, outcome) VALUES (?,?,?,?,?,?)`, + file, "go", string(CallSiteCall), "Execute", "x", int(bucketReceiver)); err != nil { + t.Fatal(err) + } + } + } + if err := tx.Commit(); err != nil { + t.Fatal(err) + } + s := &Store{db: db} + start := time.Now() + _, total, err := s.OpenCallSites(context.Background(), "Execute", "go", 0) + took := time.Since(start) + if err != nil { + t.Fatal(err) + } + if total != 2000 { + t.Fatalf("control: %d open calls, want 2000", total) + } + if took > 1500*time.Millisecond { + t.Errorf("the census of 2,000 calls over 20,000 imports took %v: its cost scales with imports per site", took) + } +} diff --git a/internal/topology/callsites_query.go b/internal/topology/callsites_query.go index 032287b0..250ac44b 100644 --- a/internal/topology/callsites_query.go +++ b/internal/topology/callsites_query.go @@ -215,14 +215,17 @@ type OpenCall struct { // declare the name once). An external call is returned for the caller to judge // by its qualifier. func (s *Store) OpenCallSites(ctx context.Context, name, language string, limit int) (open []OpenCall, total int, err error) { + dots, err := s.dotImportFiles(ctx) + if err != nil { + return nil, 0, err + } // A call counts as accounted for only while the edge it produced still stands: // a scoped pass re-reads only the changed files' sites, so a caller elsewhere // keeps "resolved" after its target is deleted or renamed, with no edge left. A // same-file call of the enclosing declaration's own name is recursion, which no // pass draws an edge for and no census counts as a caller. rows, err := s.db.QueryContext(ctx, ` - SELECT f.path, cs.start_line, IFNULL(cs.qualifier, ''), cs.outcome, - EXISTS (SELECT 1 FROM topology_nodes i WHERE i.file_id = cs.file_id AND i.kind = ? AND i.name = '.'), + SELECT f.path, cs.file_id, cs.start_line, IFNULL(cs.qualifier, ''), cs.outcome, cs.outcome IN (?, ?, ?) AND (cs.enclosing_id IS NULL OR NOT EXISTS ( SELECT 1 FROM topology_edges e JOIN topology_nodes t ON t.id = e.to_id WHERE e.from_id = cs.enclosing_id AND e.kind = ? AND t.name = cs.callee)) @@ -242,7 +245,6 @@ func (s *Store) OpenCallSites(ctx context.Context, name, language string, limit AND NOT (cs.outcome = ? AND EXISTS ( SELECT 1 FROM topology_nodes en WHERE en.id = cs.enclosing_id AND en.name = cs.callee)))) ORDER BY f.path, cs.start_byte`, - string(KindImport), int(bucketResolved), int(bucketRepeat), int(bucketSameFile), string(EdgeCalls), int(bucketSameFile), int(bucketSameFile), string(KindFunction), string(KindMethod), string(KindTest), language, name, string(CallSiteCall), @@ -257,9 +259,11 @@ func (s *Store) OpenCallSites(ctx context.Context, name, language string, limit var c OpenCall var outcome int var dangling, ambiguous bool - if err := rows.Scan(&c.Path, &c.Line, &c.Qualifier, &outcome, &c.DotImport, &dangling, &ambiguous); err != nil { + var fileID int64 + if err := rows.Scan(&c.Path, &fileID, &c.Line, &c.Qualifier, &outcome, &dangling, &ambiguous); err != nil { return nil, 0, fmt.Errorf("topology: open call sites: %w", err) } + c.DotImport = dots[fileID] c.Kind = openKind(resolveBucket(outcome)) if dangling && !ambiguous { c.Kind = "stale" @@ -337,3 +341,24 @@ func (s *Store) UnreadFiles(ctx context.Context, ext string) (int, error) { } return n, nil } + +// dotImportFiles is the set of files that dot-import a package, read once per +// census. Asked per call site instead, the planner reaches for the import-kind +// index and scans every import node of the workspace for each site: on this +// repository a common method name's census took seconds (PLAN-544). +func (s *Store) dotImportFiles(ctx context.Context) (map[int64]bool, error) { + rows, err := s.db.QueryContext(ctx, `SELECT DISTINCT file_id FROM topology_nodes WHERE name = '.' AND kind = ?`, string(KindImport)) + if err != nil { + return nil, fmt.Errorf("topology: dot imports: %w", err) + } + defer rows.Close() + out := map[int64]bool{} + for rows.Next() { + var id int64 + if err := rows.Scan(&id); err != nil { + return nil, fmt.Errorf("topology: dot imports: %w", err) + } + out[id] = true + } + return out, rows.Err() +}