diff --git a/cmd/gortex/daemon.go b/cmd/gortex/daemon.go index 4e88b863..190598b6 100644 --- a/cmd/gortex/daemon.go +++ b/cmd/gortex/daemon.go @@ -209,6 +209,16 @@ func runDaemonStart(cmd *cobra.Command, _ []string) error { return fmt.Errorf("build daemon state: %w", err) } + // Install the standing soft memory limit now — logging and config are + // up and no warmup / indexing has allocated yet, so the cold-index + // window's temporary override restores to this value rather than to + // "no limit" (see applyStandingMemoryLimit). + var daemonMemLimit string + if gc := state.configManager.Global(); gc != nil { + daemonMemLimit = gc.Daemon.MemoryLimit + } + applyStandingMemoryLimit(logger, daemonMemLimit) + controller := &realController{ graph: state.graph, indexer: state.indexer, @@ -603,6 +613,15 @@ func runDaemonStart(cmd *cobra.Command, _ []string) error { "warmup_ms": elapsed.Milliseconds(), }) logWarmupSummary(logger, warmup, queryableElapsed, elapsed) + // Warmup is the daemon's single largest allocation burst (parse + + // resolve + the end_batch graph passes, then the whole-graph + // analysis above). This is the last point reached in every warmup + // shape — every early return inside warmupDaemonState still lands + // here — so return the burst's heap high-water to the OS now rather + // than letting the peak pin the idle footprint. RunAnalysis above + // may already have released, but the enrichment tail allocates + // further, so a final release at the true end still pays. + releaseMemoryToOS(logger, "warmup_complete") }() return srv.Serve() @@ -657,11 +676,29 @@ func startReconcileJanitor(mi *indexer.MultiIndexer, interval time.Duration, log for { select { case <-t.C: - if gced := mi.GCVanishedWorktrees(); len(gced) > 0 { + gced := mi.GCVanishedWorktrees() + if len(gced) > 0 { logger.Info("janitor: pruned vanished worktrees", zap.Int("count", len(gced))) } - mi.ReconcileAll() + results := mi.ReconcileAll() + // Return the tick's heap to the OS only when it actually did + // work — a repo reindexed stale/deleted files, or a worktree + // was pruned. ReconcileAll fills a result for every repo, so + // the honest "did work" signal is the per-repo stale/deleted + // counts, not the map size. A quiescent tick skips the + // release: FreeOSMemory is a full GC, and paying it hourly + // for a no-op sweep is the periodic cost the release policy + // exists to avoid. + reconciled := 0 + for _, r := range results { + if r != nil { + reconciled += r.StaleFileCount + r.DeletedFileCount + } + } + if reconciled > 0 || len(gced) > 0 { + releaseMemoryToOS(logger, "reconcile_janitor") + } case <-stop: return } diff --git a/cmd/gortex/daemon_memlimit.go b/cmd/gortex/daemon_memlimit.go new file mode 100644 index 00000000..7821472b --- /dev/null +++ b/cmd/gortex/daemon_memlimit.go @@ -0,0 +1,279 @@ +package main + +import ( + "fmt" + "math" + "os" + "runtime" + "runtime/debug" + "strconv" + "strings" + "time" + + "go.uber.org/zap" + + "github.com/zzet/gortex/internal/platform" +) + +// Daemon memory envelope: a standing soft memory limit installed at boot, +// and a forced heap-to-OS release fired at allocation-burst boundaries. +// +// The daemon is a long-lived background service that shares a developer's +// machine. With the runtime default (no memory limit, GOGC=100) the Go GC +// lets the heap high-water climb toward machine RAM during a burst and then +// keeps that footprint resident — the observed failure was a multi-GB peak +// from a warmup / whole-graph-analysis burst pinning the process footprint +// for hours at idle. A soft memory limit makes the collector pace against a +// ceiling and resist that balloon growth; the release helper returns a +// burst's high-water to the OS so the idle footprint tracks the working set +// rather than the peak. Both are policy, not hard guarantees, and both carry +// env kill-switches so an operator can bypass them entirely. + +const ( + // standingMemLimitDivisor takes a quarter of host RAM as the default + // budget: a background service should leave the bulk of RAM to the + // editor, compiler, and the app under test. + standingMemLimitDivisor = 4 + // standingMemLimitFloor is the lowest limit the default policy will + // set. Below ~1 GiB a daemon with a resident graph would sit in + // near-constant GC, so the floor trades a little footprint for + // steady-state responsiveness. + standingMemLimitFloor = int64(1) << 30 // 1 GiB + // standingMemLimitCeil caps the default so a large workstation's + // quarter-RAM figure doesn't hand the daemon a tens-of-GiB budget it + // never needs — the point of the limit is to resist balloon growth, + // not to permit it. An operator who genuinely wants more sets an + // explicit value (honored verbatim, below). + standingMemLimitCeil = int64(8) << 30 // 8 GiB +) + +// memLimitResolution is the outcome of the standing-limit policy: the byte +// value to install (0 = install nothing), where it came from (for the log +// line), and an optional warning when a provided value was malformed and +// ignored. +type memLimitResolution struct { + limit int64 + source string // "goenv" | "env" | "config" | "default" | "off" | "unavailable" + warn string +} + +// resolveStandingMemoryLimit is the pure standing-limit policy. Resolution +// order, highest priority first: +// +// 1. GOMEMLIMIT set — the Go runtime already honors it, so we never fight +// an explicit operator setting: install nothing, source "goenv". +// 2. GORTEX_DAEMON_MEMLIMIT env var. +// 3. the daemon.memory_limit config value. +// 4. the RAM-derived default policy. +// +// An explicit env/config value is honored verbatim (the operator knows +// their machine); only the default is clamped. "off"/"0" at the env or +// config layer disables the standing limit outright. A malformed env/config +// value is ignored and the decision falls through to the default with a +// warning. Pure — every input is a parameter — so the whole precedence +// table is exhaustively testable without touching the process or the host. +func resolveStandingMemoryLimit(hostRAM uint64, goenv, env, cfg string) memLimitResolution { + if strings.TrimSpace(goenv) != "" { + return memLimitResolution{source: "goenv"} + } + for _, layer := range []struct{ name, val string }{ + {"env", env}, + {"config", cfg}, + } { + v := strings.TrimSpace(layer.val) + if v == "" { + continue + } + n, err := parseByteSize(v) + if err != nil { + // A typo'd operator value should not abort boot; apply the safe + // default instead and surface why. Deliberately does not fall + // through to the next layer — a malformed value is a signal, not + // an invitation to keep guessing. + d := defaultMemLimitResolution(hostRAM) + d.warn = fmt.Sprintf("invalid %s memory limit %q: %v", layer.name, layer.val, err) + return d + } + if n <= 0 { + return memLimitResolution{source: "off"} + } + return memLimitResolution{limit: n, source: layer.name} + } + return defaultMemLimitResolution(hostRAM) +} + +// defaultMemLimitResolution wraps the RAM-derived default. Host RAM of 0 +// means "unknown" (no portable reader on this platform, or the syscall +// failed): with no machine to reason about, installing an arbitrary limit +// could throttle a large server or over-commit a tiny one, so the safe +// answer is to install nothing. +func defaultMemLimitResolution(hostRAM uint64) memLimitResolution { + n := defaultStandingMemoryLimit(hostRAM) + if n <= 0 { + return memLimitResolution{source: "unavailable"} + } + return memLimitResolution{limit: n, source: "default"} +} + +// defaultStandingMemoryLimit derives the default soft limit from host RAM: +// a quarter of it, clamped to [floor, ceil]. Returns 0 when host RAM is +// unknown. Pure so the clamps are table-testable without a host. +func defaultStandingMemoryLimit(hostRAM uint64) int64 { + if hostRAM == 0 { + return 0 + } + limit := int64(hostRAM / standingMemLimitDivisor) + if limit < standingMemLimitFloor { + limit = standingMemLimitFloor + } + if limit > standingMemLimitCeil { + limit = standingMemLimitCeil + } + return limit +} + +// parseByteSize parses a human byte size into an exact byte count. It +// accepts a bare integer (bytes) or an integer with a binary unit suffix — +// K/KB/KiB, M/MB/MiB, G/GB/GiB, T/TB/TiB — all interpreted as powers of +// 1024, case-insensitively, since this sizes a memory budget. "off", "0", +// and the empty string parse to 0 (the caller reads 0 as "disabled"). A +// malformed number, an unrecognised unit, or a value that would overflow +// int64 returns an error so the caller can fall back to the default policy. +func parseByteSize(s string) (int64, error) { + s = strings.TrimSpace(s) + if s == "" { + return 0, nil + } + low := strings.ToLower(s) + if low == "off" || low == "0" { + return 0, nil + } + i := 0 + for i < len(low) && low[i] >= '0' && low[i] <= '9' { + i++ + } + numPart := low[:i] + unit := strings.TrimSpace(low[i:]) + if numPart == "" { + return 0, fmt.Errorf("invalid byte size %q", s) + } + n, err := strconv.ParseInt(numPart, 10, 64) + if err != nil || n < 0 { + return 0, fmt.Errorf("invalid byte size %q", s) + } + var mult int64 + switch unit { + case "", "b": + mult = 1 + case "k", "kb", "kib": + mult = 1 << 10 + case "m", "mb", "mib": + mult = 1 << 20 + case "g", "gb", "gib": + mult = 1 << 30 + case "t", "tb", "tib": + mult = 1 << 40 + default: + return 0, fmt.Errorf("invalid byte-size unit %q in %q", unit, s) + } + if mult > 1 && n > math.MaxInt64/mult { + return 0, fmt.Errorf("byte size out of range %q", s) + } + return n * mult, nil +} + +// applyStandingMemoryLimit resolves and installs the daemon's standing soft +// memory limit. Call once at boot, after logging and config are up and +// before warmup starts allocating. +// +// Composition with the cold-index window (internal/indexer/gc_tune.go): a +// cold index briefly raises the limit to a larger budget (RAM/2) and, on +// exit, restores the value it captured via debug.SetMemoryLimit(-1) — which +// is exactly the standing limit installed here. Installing this before any +// index runs is what makes that restore land on our value rather than on +// "no limit". The two are therefore composable: the daemon holds a modest +// standing ceiling, cold indexes get their wider one-shot budget, and the +// standing ceiling comes back afterward untouched. +func applyStandingMemoryLimit(logger *zap.Logger, cfgVal string) { + d := resolveStandingMemoryLimit( + platform.HostPhysicalMemoryBytes(), + os.Getenv("GOMEMLIMIT"), + os.Getenv("GORTEX_DAEMON_MEMLIMIT"), + cfgVal, + ) + if d.warn != "" && logger != nil { + logger.Warn("daemon: standing memory limit — falling back to default", + zap.String("reason", d.warn)) + } + switch d.source { + case "goenv": + if logger != nil { + logger.Info("daemon: standing memory limit deferred to GOMEMLIMIT") + } + return + case "off": + if logger != nil { + logger.Info("daemon: standing memory limit disabled by configuration") + } + return + case "unavailable": + if logger != nil { + logger.Debug("daemon: standing memory limit skipped — host RAM unknown") + } + return + } + debug.SetMemoryLimit(d.limit) + if logger != nil { + logger.Info("daemon: standing memory limit applied", + zap.Int64("bytes", d.limit), + zap.String("source", d.source)) + } +} + +// memReleaseEnabled reports whether post-burst heap release is active. On by +// default; GORTEX_DAEMON_MEMRELEASE=0 (or "false") disables it. +func memReleaseEnabled() bool { + v := os.Getenv("GORTEX_DAEMON_MEMRELEASE") + return v != "0" && !strings.EqualFold(v, "false") +} + +// releaseMemoryToOS forces a GC + scavenge (runtime/debug.FreeOSMemory) so a +// just-completed allocation burst's high-water heap is returned to the OS +// promptly instead of pinning the process footprint at its peak. +// +// It is called only at burst boundaries (warmup end, the reconcile janitor +// after a tick that did work), never on a timer: FreeOSMemory runs a full, +// largely stop-the-world GC cycle costing ~0.1–2 s on a multi-GB heap, so +// paying it once per burst is fine while paying it periodically would +// reintroduce exactly the steady-state GC cost the standing limit is tuned +// to avoid. HeapReleased is monotonic across the forced scavenge, so the +// logged delta is the bytes this call handed back. +// +// GORTEX_DAEMON_MEMRELEASE=0 (or "false") turns it into a no-op. +func releaseMemoryToOS(logger *zap.Logger, reason string) { + if !memReleaseEnabled() { + return + } + var before, after runtime.MemStats + runtime.ReadMemStats(&before) + start := time.Now() + debug.FreeOSMemory() + elapsed := time.Since(start) + runtime.ReadMemStats(&after) + // Concurrent allocation between the two reads can re-acquire released + // pages faster than this call released them, making the raw delta + // negative; report that as zero net release rather than a nonsense + // negative byte count. + freed := int64(after.HeapReleased) - int64(before.HeapReleased) + if freed < 0 { + freed = 0 + } + if logger != nil { + logger.Info("daemon: released heap to OS", + zap.String("reason", reason), + zap.Duration("elapsed", elapsed), + zap.Int64("freed_bytes", freed), + zap.Uint64("heap_sys_bytes", after.HeapSys), + zap.Uint64("heap_released_bytes", after.HeapReleased)) + } +} diff --git a/cmd/gortex/daemon_memlimit_test.go b/cmd/gortex/daemon_memlimit_test.go new file mode 100644 index 00000000..d4c74b0f --- /dev/null +++ b/cmd/gortex/daemon_memlimit_test.go @@ -0,0 +1,178 @@ +package main + +import ( + "runtime" + "testing" + + "go.uber.org/zap" +) + +const ( + kib = int64(1) << 10 + mib = int64(1) << 20 + gib = int64(1) << 30 + tib = int64(1) << 40 +) + +func TestParseByteSize(t *testing.T) { + tests := []struct { + in string + want int64 + wantErr bool + }{ + {"", 0, false}, + {"0", 0, false}, + {"off", 0, false}, + {"OFF", 0, false}, + {" off ", 0, false}, + {"1024", 1024, false}, // bare number = bytes + {"1B", 1, false}, + {"1KiB", kib, false}, + {"1kb", kib, false}, // KB treated as binary (memory budget) + {"2K", 2 * kib, false}, + {"4096MiB", 4096 * mib, false}, // == 4 GiB + {"4GiB", 4 * gib, false}, + {"2G", 2 * gib, false}, + {"2gb", 2 * gib, false}, + {"1T", tib, false}, + {"1TiB", tib, false}, + {"nonsense", 0, true}, + {"12x", 0, true}, // unknown unit + {"GiB", 0, true}, // no number + {"-5GiB", 0, true}, // negative + {"99999999999999999999G", 0, true}, // overflow + } + for _, tc := range tests { + got, err := parseByteSize(tc.in) + if tc.wantErr { + if err == nil { + t.Errorf("parseByteSize(%q): expected error, got %d", tc.in, got) + } + continue + } + if err != nil { + t.Errorf("parseByteSize(%q): unexpected error %v", tc.in, err) + continue + } + if got != tc.want { + t.Errorf("parseByteSize(%q) = %d, want %d", tc.in, got, tc.want) + } + } +} + +func TestDefaultStandingMemoryLimit(t *testing.T) { + tests := []struct { + name string + hostRAM uint64 + want int64 + }{ + {"unknown host RAM => 0", 0, 0}, + {"tiny host clamps up to floor", uint64(2) * uint64(gib), standingMemLimitFloor}, // 2GiB/4=512MiB -> 1GiB + {"mid host uses quarter", uint64(16) * uint64(gib), 4 * gib}, // 16GiB/4=4GiB + {"large host clamps down to ceil", uint64(64) * uint64(gib), standingMemLimitCeil}, // 64GiB/4=16GiB -> 8GiB + {"exactly floor boundary", uint64(4) * uint64(gib), gib}, // 4GiB/4=1GiB + {"exactly ceil boundary", uint64(32) * uint64(gib), 8 * gib}, // 32GiB/4=8GiB + } + for _, tc := range tests { + if got := defaultStandingMemoryLimit(tc.hostRAM); got != tc.want { + t.Errorf("%s: defaultStandingMemoryLimit(%d) = %d, want %d", tc.name, tc.hostRAM, got, tc.want) + } + } +} + +func TestResolveStandingMemoryLimit(t *testing.T) { + const host = uint64(16) * uint64(1<<30) // 16 GiB -> default 4 GiB + tests := []struct { + name string + hostRAM uint64 + goenv string + env string + cfg string + wantLimit int64 + wantSource string + wantWarn bool + }{ + {"GOMEMLIMIT wins, install nothing", host, "5GiB", "4GiB", "2GiB", 0, "goenv", false}, + {"GOMEMLIMIT wins even when malformed-looking", host, "someval", "4GiB", "", 0, "goenv", false}, + {"env value honored verbatim", host, "", "6GiB", "2GiB", 6 * gib, "env", false}, + {"env off disables", host, "", "off", "2GiB", 0, "off", false}, + {"env zero disables", host, "", "0", "2GiB", 0, "off", false}, + {"config used when env empty", host, "", "", "3GiB", 3 * gib, "config", false}, + {"config off disables", host, "", "", "off", 0, "off", false}, + {"default when nothing set", host, "", "", "", 4 * gib, "default", false}, + {"malformed env falls back to default with warning", host, "", "bogus", "2GiB", 4 * gib, "default", true}, + {"malformed config falls back to default with warning", host, "", "", "bogus", 4 * gib, "default", true}, + {"default clamps up on tiny host", uint64(2) * uint64(gib), "", "", "", standingMemLimitFloor, "default", false}, + {"default clamps down on large host", uint64(128) * uint64(gib), "", "", "", standingMemLimitCeil, "default", false}, + {"unknown host and no explicit value", 0, "", "", "", 0, "unavailable", false}, + } + for _, tc := range tests { + got := resolveStandingMemoryLimit(tc.hostRAM, tc.goenv, tc.env, tc.cfg) + if got.limit != tc.wantLimit || got.source != tc.wantSource { + t.Errorf("%s: resolve(...) = {limit:%d, source:%q}, want {limit:%d, source:%q}", + tc.name, got.limit, got.source, tc.wantLimit, tc.wantSource) + } + if (got.warn != "") != tc.wantWarn { + t.Errorf("%s: warn=%q, wantWarn=%v", tc.name, got.warn, tc.wantWarn) + } + } +} + +func TestMemReleaseEnabled(t *testing.T) { + tests := []struct { + val string + want bool + }{ + {"", true}, // unset => on by default + {"1", true}, + {"true", true}, + {"anything", true}, + {"0", false}, // kill-switch + {"false", false}, + {"FALSE", false}, + } + for _, tc := range tests { + t.Setenv("GORTEX_DAEMON_MEMRELEASE", tc.val) + if got := memReleaseEnabled(); got != tc.want { + t.Errorf("memReleaseEnabled() with %q = %v, want %v", tc.val, got, tc.want) + } + } +} + +func TestReleaseMemoryToOS_KillSwitch(t *testing.T) { + // With the kill-switch set, the helper must be a no-op: it must not + // panic and must return without forcing a collection. We can only + // observe "no crash / returns" here; the enablement predicate is + // covered exhaustively by TestMemReleaseEnabled. + t.Setenv("GORTEX_DAEMON_MEMRELEASE", "0") + releaseMemoryToOS(zap.NewNop(), "kill-switch-test") +} + +func TestReleaseMemoryToOS_Smoke(t *testing.T) { + t.Setenv("GORTEX_DAEMON_MEMRELEASE", "1") + + // Allocate then drop a chunk so there is heap for the forced scavenge + // to return, then confirm HeapReleased did not go backwards across the + // call. Exact byte counts are intentionally not asserted. + sink := make([]byte, 8<<20) + for i := range sink { + sink[i] = byte(i) + } + // Force the writes to land (so the allocation is not elided), then let the + // reference drop here: sink is dead past this point, so the scavenge below + // has heap to return. + runtime.KeepAlive(sink) + + var before runtime.MemStats + runtime.ReadMemStats(&before) + + releaseMemoryToOS(zap.NewNop(), "smoke-test") + + var after runtime.MemStats + runtime.ReadMemStats(&after) + + if after.HeapReleased < before.HeapReleased { + t.Fatalf("HeapReleased went backwards after FreeOSMemory: before=%d after=%d", + before.HeapReleased, after.HeapReleased) + } +} diff --git a/docs/multi-repo.md b/docs/multi-repo.md index 1364f893..d825407b 100644 --- a/docs/multi-repo.md +++ b/docs/multi-repo.md @@ -80,6 +80,8 @@ Environment variables: - `GORTEX_WARMUP_FULL_RETRACK=1` — force every repo through a whole-repo re-track (evict + re-parse every file) on the next warm restart instead of the default scoped reconcile. An escape hatch for when the on-disk change census itself is suspect. - `GORTEX_WARMUP_FULL_RESOLVE=1` — force the warm-restart master resolve to re-examine the whole graph instead of scoping to changed repos; also makes the resolver ignore the durable terminal-edge stamp and re-attempt every previously-given-up-on edge. Use when a scoped resolve is suspected of missing edges. - `GORTEX_WARMUP_FORCE_ENRICH=1` — bypass the persisted per-repo enrichment-completion markers and re-run semantic enrichment for every repo on warm restart, even ones whose marker already matches HEAD on a clean tree. +- `GORTEX_DAEMON_MEMLIMIT` — standing soft memory limit installed at daemon boot, as a human size (`4GiB`, `2048MiB`, `2G`) or `off` / `0` to disable. The daemon is a long-lived background service; a soft limit makes the GC pace against a ceiling and resist heap balloon growth rather than letting the high-water climb toward machine RAM. Overrides the `daemon.memory_limit` config value; an explicit `GOMEMLIMIT` overrides both (the runtime already honors it). Unset applies the default policy: a quarter of host RAM, clamped to `[1GiB, 8GiB]`. The cold-index window temporarily raises this to a larger budget and restores it afterward. +- `GORTEX_DAEMON_MEMRELEASE=0` — disable the post-burst heap-to-OS release. By default the daemon calls `debug.FreeOSMemory()` at allocation-burst boundaries (warmup completion, a reconcile-janitor tick that reindexed something, the close of a cold-index window, and a whole-graph analysis pass) so a burst's high-water footprint is returned to the OS promptly instead of pinning resident memory at the peak. It only ever fires at those boundaries, never on a timer. ## CLI diff --git a/internal/config/gitignore_cache.go b/internal/config/gitignore_cache.go new file mode 100644 index 00000000..e0e8541f --- /dev/null +++ b/internal/config/gitignore_cache.go @@ -0,0 +1,162 @@ +package config + +import ( + "os" + "path/filepath" + "sync" + "time" +) + +// gitignoreStat is the cheap staleness key for a repo's root `.gitignore`. +// EffectiveExclude runs on a firehose of calls (every indexer walk and +// per-file reconcile); statting is a single syscall that lets the cache +// skip the open+scan+allocate work unless the file actually changed. +// +// mtime+size is the standard staleness heuristic: the only edit it cannot +// see is a same-second, same-byte-size, different-content rewrite, which is +// astronomically rare in practice and self-heals on the next mtime tick. +type gitignoreStat struct { + exists bool + modTime time.Time + size int64 +} + +// equal compares two stat keys. time.Time is compared with Equal (by +// instant) rather than ==, which would also weigh monotonic-clock and +// location and can report unequal for two readings of the same timestamp. +func (s gitignoreStat) equal(o gitignoreStat) bool { + return s.exists == o.exists && s.size == o.size && s.modTime.Equal(o.modTime) +} + +// statGitignore returns the current staleness key for repoPath's root +// `.gitignore`. A missing or unstattable file yields the zero +// (exists=false) key, matching loadRepoGitignore's "absent → nil" contract. +func statGitignore(repoPath string) gitignoreStat { + if repoPath == "" { + return gitignoreStat{} + } + info, err := os.Stat(filepath.Join(repoPath, ".gitignore")) + if err != nil { + return gitignoreStat{} + } + return gitignoreStat{exists: true, modTime: info.ModTime(), size: info.Size()} +} + +// gitignoreEntry caches the parsed patterns of one repo's `.gitignore` +// alongside the stat key they were parsed from. +type gitignoreEntry struct { + stat gitignoreStat + patterns []string // shared, immutable — callers must not mutate +} + +// mergedEntry caches the fully layered exclude list for one repoPrefix. +// The config pointers pin the inputs by identity: GlobalConfig and the +// per-repo workspace Config are only ever swapped (Reload / LoadWorkspace +// Config), never mutated in place, so an unchanged pointer means unchanged +// content and no rebuild is needed. +type mergedEntry struct { + gc *GlobalConfig + ws *Config + repoPath string + respect bool + giStat gitignoreStat + merged []string // clipped, shared, immutable — callers must not mutate +} + +// excludeCache memoizes the two per-call allocations ConfigManager. +// EffectiveExclude previously repeated on every invocation: +// +// - the parsed root `.gitignore`, keyed by repo root path (the dominant +// cost: a fresh 64 KiB scanner buffer plus a string per line, read on +// every call); +// - the fully layered exclude list, keyed by repoPrefix (a fresh merge +// slice per call). +// +// A steady-state call does one os.Stat and returns shared, immutable +// slices; the underlying file read and slice merge happen only when a +// config pointer or the `.gitignore` stat actually changes. +type excludeCache struct { + mu sync.RWMutex + gitignore map[string]*gitignoreEntry // repo root path → parsed .gitignore + merged map[string]*mergedEntry // repoPrefix → layered excludes + // readFn reads a repo's root `.gitignore`. nil selects loadRepoGitignore; + // tests substitute a counting reader to assert cache hits never re-read. + readFn func(string) []string +} + +// excludeCacheCap bounds each map defensively. Keys are normally the +// configured repo set (one per tracked repo), so this is never reached in +// practice; it exists only so a caller passing unbounded arbitrary paths or +// prefixes can never grow the cache without limit. On breach the map is +// cleared and rewarms — simpler than an LRU and safe for a set this small. +const excludeCacheCap = 4096 + +func newExcludeCache() *excludeCache { + return &excludeCache{ + gitignore: make(map[string]*gitignoreEntry), + merged: make(map[string]*mergedEntry), + } +} + +func (c *excludeCache) read(repoPath string) []string { + if c.readFn != nil { + return c.readFn(repoPath) + } + return loadRepoGitignore(repoPath) +} + +// patterns returns the shared, immutable parsed patterns of repoPath's +// root `.gitignore` for the given (already-computed) stat key, re-reading +// only when the file changed. An absent file returns nil without opening +// anything. +func (c *excludeCache) patterns(repoPath string, st gitignoreStat) []string { + if !st.exists { + return nil + } + c.mu.RLock() + e := c.gitignore[repoPath] + c.mu.RUnlock() + if e != nil && e.stat.equal(st) { + return e.patterns + } + patterns := c.read(repoPath) + c.mu.Lock() + if len(c.gitignore) >= excludeCacheCap { + c.gitignore = make(map[string]*gitignoreEntry) + } + c.gitignore[repoPath] = &gitignoreEntry{stat: st, patterns: patterns} + c.mu.Unlock() + return patterns +} + +// lookupMerged returns the cached layered exclude list for repoPrefix when +// it is still valid for the given config pointers, respect flag, and +// `.gitignore` stat. The returned slice is shared — callers must not mutate. +func (c *excludeCache) lookupMerged(repoPrefix string, gc *GlobalConfig, ws *Config, repoPath string, respect bool, st gitignoreStat) ([]string, bool) { + c.mu.RLock() + e := c.merged[repoPrefix] + c.mu.RUnlock() + if e != nil && e.gc == gc && e.ws == ws && e.repoPath == repoPath && e.respect == respect && e.giStat.equal(st) { + return e.merged, true + } + return nil, false +} + +// storeMerged records the layered exclude list for repoPrefix. merged must +// be clipped (len == cap) by the caller so a consumer appending to it is +// forced to reallocate and can never write through the shared backing array. +func (c *excludeCache) storeMerged(repoPrefix string, gc *GlobalConfig, ws *Config, repoPath string, respect bool, st gitignoreStat, merged []string) { + c.mu.Lock() + if len(c.merged) >= excludeCacheCap { + c.merged = make(map[string]*mergedEntry) + } + c.merged[repoPrefix] = &mergedEntry{ + gc: gc, + ws: ws, + repoPath: repoPath, + respect: respect, + giStat: st, + merged: merged, + } + c.mu.Unlock() +} diff --git a/internal/config/gitignore_cache_test.go b/internal/config/gitignore_cache_test.go new file mode 100644 index 00000000..a88e776d --- /dev/null +++ b/internal/config/gitignore_cache_test.go @@ -0,0 +1,272 @@ +package config + +import ( + "os" + "path/filepath" + "strings" + "sync/atomic" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/zzet/gortex/internal/excludes" +) + +func newTestConfigManager(t *testing.T) *ConfigManager { + t.Helper() + cm, err := NewConfigManager("/tmp/nonexistent-gortex-cache-test/config.yaml") + require.NoError(t, err) + return cm +} + +func writeGitignore(t *testing.T, dir, content string) { + t.Helper() + require.NoError(t, os.WriteFile(filepath.Join(dir, ".gitignore"), []byte(content), 0o644)) +} + +func writeWorkspace(t *testing.T, dir, content string) { + t.Helper() + require.NoError(t, os.WriteFile(filepath.Join(dir, ".gortex.yaml"), []byte(content), 0o644)) +} + +// bumpMtime forces a stat-visible change so invalidation fires regardless +// of the filesystem's mtime resolution. +func bumpMtime(t *testing.T, path string) { + t.Helper() + future := time.Now().Add(2 * time.Second) + require.NoError(t, os.Chtimes(path, future, future)) +} + +// countingReader wires a call counter into the cache's file reader so a +// test can assert that a cache hit never re-opens the `.gitignore`. +func countingReader(cm *ConfigManager) *int64 { + var reads int64 + cm.excludeCache.readFn = func(p string) []string { + atomic.AddInt64(&reads, 1) + return loadRepoGitignore(p) + } + return &reads +} + +func TestExcludeCache_CacheHitDoesNotReReadGitignore(t *testing.T) { + cm := newTestConfigManager(t) + repoDir := t.TempDir() + writeGitignore(t, repoDir, "node_modules/\n*.log\n") + cm.LoadWorkspaceConfig("r", repoDir) + + reads := countingReader(cm) + + first := cm.EffectiveExclude("r") + second := cm.EffectiveExclude("r") + + assert.Contains(t, first, "node_modules/") + assert.Contains(t, second, "node_modules/") + assert.Contains(t, second, "*.log") + assert.Equal(t, first, second, "cache hit must return the same layered list") + assert.Equal(t, int64(1), atomic.LoadInt64(reads), + "the second call must be a cache hit, not a re-read") +} + +func TestExcludeCache_IdenticalMatchingColdVsWarm(t *testing.T) { + cm := newTestConfigManager(t) + repoDir := t.TempDir() + writeGitignore(t, repoDir, "build/\n*.tmp\ndata/raw/\n") + cm.LoadWorkspaceConfig("r", repoDir) + + cases := []struct { + path string + exclude bool + }{ + {"build/output.o", true}, + {"src/main.go", false}, + {"scratch.tmp", true}, + {"data/raw/big.bin", true}, + {"data/clean/ok.csv", false}, + {".git/config", true}, // builtin baseline + {"README.md", false}, + } + + assertMatches := func(list []string, label string) { + m := excludes.New(list) + for _, c := range cases { + assert.Equalf(t, c.exclude, m.MatchRel(c.path), + "%s: path %q", label, c.path) + } + } + + assertMatches(cm.EffectiveExclude("r"), "cold") // populates the cache + assertMatches(cm.EffectiveExclude("r"), "warm") // cache hit +} + +func TestExcludeCache_InvalidatesOnGitignoreChange(t *testing.T) { + cm := newTestConfigManager(t) + repoDir := t.TempDir() + writeGitignore(t, repoDir, "old-only/\n") + cm.LoadWorkspaceConfig("r", repoDir) + + reads := countingReader(cm) + + first := cm.EffectiveExclude("r") + assert.Contains(t, first, "old-only/") + + // Different content (also a different byte size) plus a bumped mtime so + // the change is detected even on a coarse-resolution filesystem. + writeGitignore(t, repoDir, "new-only/\nsecond-new/\n") + bumpMtime(t, filepath.Join(repoDir, ".gitignore")) + + second := cm.EffectiveExclude("r") + assert.Contains(t, second, "new-only/", "new patterns must take effect after the edit") + assert.Contains(t, second, "second-new/") + assert.NotContains(t, second, "old-only/", "stale patterns must be dropped") + assert.Equal(t, int64(2), atomic.LoadInt64(reads), + "the changed file must be re-read exactly once") +} + +func TestExcludeCache_InvalidatesOnWorkspaceReload(t *testing.T) { + cm := newTestConfigManager(t) + repoDir := t.TempDir() + writeWorkspace(t, repoDir, "exclude:\n - \"first/**\"\n") + cm.LoadWorkspaceConfig("r", repoDir) + + first := cm.EffectiveExclude("r") + assert.Contains(t, first, "first/**") + assert.NotContains(t, first, "second/**") + + // Reloading swaps the cached *Config pointer, which must invalidate the + // merged entry even though the `.gitignore` (absent) is unchanged. + writeWorkspace(t, repoDir, "exclude:\n - \"second/**\"\n") + cm.LoadWorkspaceConfig("r", repoDir) + + second := cm.EffectiveExclude("r") + assert.Contains(t, second, "second/**", "workspace edit must be reflected") + assert.NotContains(t, second, "first/**", "stale workspace patterns must be dropped") +} + +func TestExcludeCache_NoGitignoreNeverReads(t *testing.T) { + cm := newTestConfigManager(t) + repoDir := t.TempDir() + cm.LoadWorkspaceConfig("r", repoDir) + + reads := countingReader(cm) + + got := cm.EffectiveExclude("r") + _ = cm.EffectiveExclude("r") + + assert.Equal(t, excludes.Builtin, got, "absent .gitignore yields the builtin baseline") + assert.Equal(t, int64(0), atomic.LoadInt64(reads), + "an absent .gitignore must not trigger a file read") +} + +func TestExcludeCache_ReturnedSliceIsClippedAndAppendSafe(t *testing.T) { + cm := newTestConfigManager(t) + repoDir := t.TempDir() + writeGitignore(t, repoDir, "z-marker/\n") + cm.LoadWorkspaceConfig("r", repoDir) + + first := cm.EffectiveExclude("r") + require.Equal(t, len(first), cap(first), + "returned slice must be clipped so append reallocates") + + // Appending to the returned slice must not disturb the shared cached + // value handed to every other reader. + _ = append(first, "mutant/**") //nolint:staticcheck // intentional: exercise append safety + + second := cm.EffectiveExclude("r") + assert.NotContains(t, second, "mutant/**", + "appending to the returned slice must not corrupt the cache") + assert.Equal(t, first, second) +} + +// uncachedEffectiveExclude reproduces the pre-cache EffectiveExclude body — +// a fresh `.gitignore` read plus a fresh merge on every call — so the +// benchmark can quote the baseline the cache replaces. +func uncachedEffectiveExclude(gc *GlobalConfig, ws *Config, repoPrefix, repoPath string) []string { + out := make([]string, 0, 32) + out = append(out, excludes.Builtin...) + if shouldRespectGitignore(ws) && repoPath != "" { + out = append(out, loadRepoGitignore(repoPath)...) + } + if gc != nil { + out = append(out, gc.Exclude...) + if entry := gc.FindRepoByPrefix(repoPrefix); entry != nil { + out = append(out, entry.Exclude...) + } + } + if ws != nil { + out = append(out, ws.Exclude...) + if len(ws.Exclude) == 0 { + out = append(out, ws.Index.Exclude...) + out = append(out, ws.Watch.Exclude...) + } + } + if ws != nil { + for _, inc := range ws.Include { + inc = strings.TrimSpace(inc) + if inc == "" { + continue + } + if !strings.HasPrefix(inc, "!") { + inc = "!" + inc + } + out = append(out, inc) + } + } + return out +} + +const benchGitignore = `# build output +build/ +dist/ +*.log +*.tmp +node_modules/ +vendor/ +coverage/ +.cache/ +target/ +__pycache__/ +*.pyc +.venv/ +data/raw/ +data/interim/ +testdata/large/ +*.bin +*.gz +.DS_Store +` + +func benchSetup(b *testing.B) *ConfigManager { + b.Helper() + cm, err := NewConfigManager("/tmp/nonexistent-gortex-cache-bench/config.yaml") + require.NoError(b, err) + repoDir := b.TempDir() + require.NoError(b, os.WriteFile(filepath.Join(repoDir, ".gitignore"), []byte(benchGitignore), 0o644)) + cm.LoadWorkspaceConfig("r", repoDir) + return cm +} + +func BenchmarkEffectiveExclude(b *testing.B) { + cm := benchSetup(b) + cm.EffectiveExclude("r") // warm the cache + b.ReportAllocs() + b.ResetTimer() + for i := 0; i < b.N; i++ { + _ = cm.EffectiveExclude("r") + } +} + +func BenchmarkEffectiveExcludeUncached(b *testing.B) { + cm := benchSetup(b) + cm.mu.RLock() + gc := cm.global + ws := cm.workspace["r"] + repoPath := cm.workspacePaths["r"] + cm.mu.RUnlock() + b.ReportAllocs() + b.ResetTimer() + for i := 0; i < b.N; i++ { + _ = uncachedEffectiveExclude(gc, ws, "r", repoPath) + } +} diff --git a/internal/config/global.go b/internal/config/global.go index c6e79ce0..7d77bc47 100644 --- a/internal/config/global.go +++ b/internal/config/global.go @@ -84,10 +84,26 @@ type GlobalConfig struct { // across repos instead of being duplicated in every `.gortex.yaml`. Embedding EmbeddingConfig `mapstructure:"embedding" yaml:"embedding,omitempty"` + // Daemon carries policy for the long-running daemon process itself — + // settings that govern the shared background service rather than any + // single workspace. Lives in the user-level config for that reason. + Daemon DaemonConfig `mapstructure:"daemon" yaml:"daemon,omitempty"` + // configPath stores the file path used for Save(). Set by LoadGlobal or SetConfigPath. configPath string `yaml:"-"` } +// DaemonConfig is the `daemon:` block in ~/.gortex/config.yaml. +type DaemonConfig struct { + // MemoryLimit is the standing soft memory limit (the Go runtime's + // SetMemoryLimit) applied at daemon boot, written as a human size — + // "4GiB", "2048MiB", "2G" — or "off" / "0" to disable it. Empty + // applies the built-in default policy (a fraction of host RAM, clamped + // to a sane band). The GORTEX_DAEMON_MEMLIMIT env var overrides this, + // and a runtime-honored GOMEMLIMIT overrides both. + MemoryLimit string `mapstructure:"memory_limit" yaml:"memory_limit,omitempty"` +} + // MergeLLMInto layers a repo-local llm.Config over the global user // config: each zero-valued field of local is filled from gc.LLM, // per provider sub-block. Local non-zero values always win — including diff --git a/internal/config/manager.go b/internal/config/manager.go index 9b7d467e..1f104e65 100644 --- a/internal/config/manager.go +++ b/internal/config/manager.go @@ -27,6 +27,10 @@ type ConfigManager struct { workspacePaths map[string]string mu sync.RWMutex logger *zap.Logger + // excludeCache memoizes the per-repo `.gitignore` parse and the layered + // exclude list so EffectiveExclude — called on every indexer walk and + // per-file reconcile — does not re-read and re-merge on every call. + excludeCache *excludeCache } // NewConfigManager creates a ConfigManager by loading the GlobalConfig @@ -49,6 +53,7 @@ func NewConfigManager(globalPath string) (*ConfigManager, error) { workspace: make(map[string]*Config), workspacePaths: make(map[string]string), logger: zap.NewNop(), + excludeCache: newExcludeCache(), }, nil } @@ -196,6 +201,18 @@ func (cm *ConfigManager) GetRepoConfig(repoPrefix string) *Config { // 4. Matching RepoEntry.Exclude (first match in Repos, then Projects) // 5. Workspace .gortex.yaml top-level Exclude // 6. Legacy workspace Index.Exclude / Watch.Exclude (deprecated) +// +// This runs on a firehose of calls (every indexer walk and per-file +// reconcile), so the layered result is memoized per repo and returned +// shared: a steady-state call does one os.Stat of the repo's `.gitignore` +// and returns the cached slice without re-reading or re-merging. The cache +// invalidates on config changes (the global and workspace configs are +// swapped, never mutated in place, so pointer identity is the version) and +// on a `.gitignore` mtime/size change. +// +// The returned slice is SHARED and IMMUTABLE: callers MUST NOT mutate its +// elements. It is clipped (len == cap), so appending to it is safe — +// append reallocates rather than writing through the shared backing array. func (cm *ConfigManager) EffectiveExclude(repoPrefix string) []string { cm.mu.RLock() gc := cm.global @@ -203,16 +220,24 @@ func (cm *ConfigManager) EffectiveExclude(repoPrefix string) []string { repoPath := cm.workspacePaths[repoPrefix] cm.mu.RUnlock() + respect := shouldRespectGitignore(ws) + var st gitignoreStat + if respect && repoPath != "" { + st = statGitignore(repoPath) + } + if m, ok := cm.excludeCache.lookupMerged(repoPrefix, gc, ws, repoPath, respect, st); ok { + return m + } + out := make([]string, 0, 32) out = append(out, excludes.Builtin...) // Layer 2: repo `.gitignore`, unless the workspace config explicitly - // opts out. Reading happens on every EffectiveExclude call — the - // file is tiny and the function isn't on a hot path; refreshing - // every read keeps mid-session edits to `.gitignore` picked up - // without needing to wire cache invalidation. - if shouldRespectGitignore(ws) && repoPath != "" { - out = append(out, loadRepoGitignore(repoPath)...) + // opts out. The parse is cached per repo path and refreshed only when + // the file's mtime/size changes (see excludeCache), so a mid-session + // edit is still picked up on the next call. + if respect && repoPath != "" { + out = append(out, cm.excludeCache.patterns(repoPath, st)...) } if gc != nil { @@ -247,6 +272,12 @@ func (cm *ConfigManager) EffectiveExclude(repoPrefix string) []string { out = append(out, inc) } } + + // Clip to len == cap so a caller that appends to the returned slice is + // forced to reallocate and can never write through the shared backing + // array the cache hands to every reader. + out = out[:len(out):len(out)] + cm.excludeCache.storeMerged(repoPrefix, gc, ws, repoPath, respect, st, out) return out } diff --git a/internal/graph/store_sqlite/bundle_cache.go b/internal/graph/store_sqlite/bundle_cache.go index 6e3e40a8..055a2d95 100644 --- a/internal/graph/store_sqlite/bundle_cache.go +++ b/internal/graph/store_sqlite/bundle_cache.go @@ -1,21 +1,59 @@ package store_sqlite import ( + "os" "path/filepath" + "strconv" + "strings" "sync" "github.com/zzet/gortex/internal/graph" ) -// bundleCacheMaxEntries bounds how many per-node bundle entries the -// cache holds. When the cap is reached the cache is cleared wholesale -// rather than evicting individually — the entries are cheap to recompute -// (one batched fetch) and a wholesale clear keeps the bookkeeping O(1) -// and free of an LRU's per-entry overhead. The cap is generous: a +// bundleCacheDefaultMaxBytes bounds the total heap the bundle cache may +// retain across all cached entries. A count ceiling alone is unsafe: an +// entry holds a decoded node plus its full in/out edge lists, and both +// nodes and edges carry meta maps, so entry sizes span ~1 KB for a leaf +// symbol to multiple MB for a hub node with thousands of edges. A cap +// measured in entries therefore admits an unbounded BYTE footprint — a +// few thousand hub bundles can pin gigabytes. This cache serves point +// lookups on the symbol-search hot path; it is a latency optimisation, +// not a working set that needs to be resident, so a modest budget is +// right — 64 MiB holds the hot few thousand ordinary bundles while +// keeping a long-lived daemon's idle heap bounded. Override with +// GORTEX_BUNDLE_CACHE_MAX_MB= (n <= 0 disables the cache entirely). +const bundleCacheDefaultMaxBytes = 64 << 20 // 64 MiB + +// bundleCacheMaxEntries is a secondary, generous count ceiling kept +// alongside the byte budget. The byte budget is the primary bound; this +// guards the map's own structural overhead in the degenerate case of a +// flood of tiny entries (a bucket slot and pointers per entry are not +// fully reflected in a per-entry byte estimate), and keeps the +// wholesale-clear allocation predictable. It is deliberately loose: a // half-million-symbol monorepo's hottest few thousand search hits fit -// comfortably under it. +// far under it, so in normal operation the byte budget always trips +// first. const bundleCacheMaxEntries = 50000 +const ( + // bundleEntryOverhead is a coarse fixed charge per cached entry that + // is independent of the bundle's string content: the bundleCacheEntry + // wrapper, the *entry and *Node pointers, the graph.Node value's flat + // struct (its string / slice / map headers, ints, and embedded + // time.Time), and the map bucket the node id occupies. String and map + // *contents* are added on top. Over-estimating here only makes the + // cache clear sooner; it never lets the footprint overshoot the budget. + bundleEntryOverhead = 448 + // bundleEdgeOverhead is the coarse fixed charge for one *Edge in an + // in/out slice: the pointer, the slice slot, and the Edge value's flat + // struct. Edge string / map contents are added separately. + bundleEdgeOverhead = 240 + // bundleMetaEntryOverhead is the fixed per-key charge for a + // map[string]any entry (bucket slot + interface header); the key + // length and any string value length are added on top. + bundleMetaEntryOverhead = 48 +) + // bundleCacheEntry is one node's cached bundle, tagged with the package // it belongs to and the package fingerprint that was current when the // bundle was computed. The entry is served only while @@ -28,6 +66,10 @@ type bundleCacheEntry struct { pkgKey string fp uint64 bundle graph.SymbolBundle + // bytes is the entry's estimated retained size, recorded at insert so + // the running byte total can be adjusted in O(1) whenever the entry is + // dropped (invalidation or a stale read). + bytes int64 } // bundleCache is a content-addressed, package-scoped cache over @@ -46,10 +88,114 @@ type bundleCacheEntry struct { // cache; a package the daemon has never reported a fingerprint for is // always treated as a miss (conservative: never serve an unvalidated // bundle). +// +// The cache is bounded by bytes (maxBytes), not by entry count, because +// entry sizes vary by orders of magnitude with a node's edge fan-out and +// meta size. maxEntries is a secondary count ceiling only. When either +// bound would be exceeded the cache is cleared wholesale rather than +// evicting individually: entries are cheap to recompute (one batched +// fetch), and a wholesale clear keeps the bookkeeping O(1) and free of an +// LRU's per-entry ordering overhead. maxBytes <= 0 disables the cache — +// stores become no-ops and every lookup misses (reads still recompute +// live through the caller's fallback path). type bundleCache struct { mu sync.Mutex fingerprints map[string]uint64 entries map[string]*bundleCacheEntry + maxBytes int64 // byte budget (primary bound); <= 0 disables the cache + maxEntries int // count ceiling (secondary bound) + curBytes int64 // running sum of entries' estimated bytes +} + +// newBundleCache builds an empty cache with the default budgets. The byte +// budget is overridable with GORTEX_BUNDLE_CACHE_MAX_MB=; n <= 0 +// disables the cache. It starts inert (every lookup a miss) until the +// daemon supplies fingerprints. +func newBundleCache() *bundleCache { + return &bundleCache{ + fingerprints: map[string]uint64{}, + entries: map[string]*bundleCacheEntry{}, + maxBytes: bundleCacheMaxBytes(), + maxEntries: bundleCacheMaxEntries, + } +} + +// bundleCacheMaxBytes resolves the byte budget from the environment, +// falling back to the default. GORTEX_BUNDLE_CACHE_MAX_MB is read in +// mebibytes; a value <= 0 returns 0 to disable the cache, and an +// unparseable value is ignored (keeps the default). +func bundleCacheMaxBytes() int64 { + if v := strings.TrimSpace(os.Getenv("GORTEX_BUNDLE_CACHE_MAX_MB")); v != "" { + if n, err := strconv.Atoi(v); err == nil { + if n <= 0 { + return 0 + } + return int64(n) << 20 + } + } + return bundleCacheDefaultMaxBytes +} + +// bundleEntryBytes conservatively estimates a bundle's retained heap for +// the byte budget: a fixed per-entry charge plus the node's string and +// meta contents plus each in/out edge's fixed charge and its string and +// meta contents. Computed once at insert so the overflow check is a cheap +// scalar comparison. +func bundleEntryBytes(b graph.SymbolBundle) int64 { + n := int64(bundleEntryOverhead) + if b.Node != nil { + n += nodeStringBytes(b.Node) + n += metaBytes(b.Node.Meta) + } + for _, e := range b.InEdges { + n += edgeBytes(e) + } + for _, e := range b.OutEdges { + n += edgeBytes(e) + } + return n +} + +// nodeStringBytes sums the byte lengths of a node's string fields (its +// heap-backed content, on top of the fixed struct overhead counted in +// bundleEntryOverhead). +func nodeStringBytes(nd *graph.Node) int64 { + return int64(len(nd.ID) + len(nd.Name) + len(nd.QualName) + len(nd.FilePath) + + len(string(nd.Kind)) + len(nd.Language) + len(nd.RepoPrefix) + + len(nd.WorkspaceID) + len(nd.ProjectID) + len(nd.AbsoluteFilePath) + + len(nd.Origin)) +} + +// edgeBytes estimates one edge's retained heap: the fixed per-edge charge +// plus its string fields and meta contents. +func edgeBytes(e *graph.Edge) int64 { + if e == nil { + return bundleEdgeOverhead + } + n := int64(bundleEdgeOverhead) + n += int64(len(e.From) + len(e.To) + len(string(e.Kind)) + len(e.FilePath) + + len(e.ConfidenceLabel) + len(e.Origin) + len(e.Tier) + len(e.Context) + + len(e.ReturnUsage) + len(e.Via) + len(e.Alias)) + n += metaBytes(e.Meta) + return n +} + +// metaBytes estimates a meta map's retained heap: a fixed charge per key +// plus the key length and, for string values, the value length. Non-string +// values fold into the fixed charge — meta values are overwhelmingly short +// scalars, and a coarse estimate only over-counts, which is safe. +func metaBytes(m map[string]any) int64 { + if len(m) == 0 { + return 0 + } + var n int64 + for k, v := range m { + n += int64(len(k) + bundleMetaEntryOverhead) + if s, ok := v.(string); ok { + n += int64(len(s)) + } + } + return n } // SetBundleFingerprints installs the authoritative per-package @@ -70,7 +216,8 @@ func (s *Store) SetBundleFingerprints(fps map[string]uint64) { } // refresh swaps in the new fingerprint map and prunes every entry whose -// package fingerprint no longer matches. +// package fingerprint no longer matches, decrementing the running byte +// total by each dropped entry's estimated size. func (c *bundleCache) refresh(fps map[string]uint64) { c.mu.Lock() defer c.mu.Unlock() @@ -82,6 +229,7 @@ func (c *bundleCache) refresh(fps map[string]uint64) { cur, ok := fps[e.pkgKey] if !ok || cur != e.fp { delete(c.entries, id) + c.curBytes -= e.bytes } } } @@ -106,7 +254,8 @@ func bundlePackageKey(filePath string) string { // lookup returns the cached bundle for id when it is fresh — the entry // exists and its package fingerprint still matches the current one. A // node whose package has no reported fingerprint is never served (ok is -// false) so an unvalidated bundle can never escape the cache. +// false) so an unvalidated bundle can never escape the cache. A stale +// entry is dropped in place and its bytes reclaimed. func (c *bundleCache) lookup(id string) (graph.SymbolBundle, bool) { c.mu.Lock() defer c.mu.Unlock() @@ -117,8 +266,9 @@ func (c *bundleCache) lookup(id string) (graph.SymbolBundle, bool) { cur, ok := c.fingerprints[e.pkgKey] if !ok || cur != e.fp { // Stale or unvalidated — drop it so a later refresh doesn't - // have to. + // have to, and reclaim its bytes. delete(c.entries, id) + c.curBytes -= e.bytes return graph.SymbolBundle{}, false } return e.bundle, true @@ -127,8 +277,14 @@ func (c *bundleCache) lookup(id string) (graph.SymbolBundle, bool) { // store records a freshly computed bundle, tagged with its package's // current fingerprint. A node whose package has no reported fingerprint // is NOT cached (it could not be validated on read-back), keeping the -// cache conservative. When the cap is exceeded the cache is cleared -// wholesale before the insert. +// cache conservative. The cache is bounded by bytes: when admitting the +// new entry would push the running total over the byte budget (or the +// count over the secondary ceiling) the cache is cleared wholesale +// before the insert. A single bundle that on its own exceeds the whole +// budget — a hub node with thousands of edges, exactly the pathological +// case a byte cap exists to keep out of long-lived memory — is refused +// outright rather than pinned. With maxBytes <= 0 the cache is disabled +// and every store is a no-op. func (c *bundleCache) store(b graph.SymbolBundle) { if b.Node == nil { return @@ -136,12 +292,29 @@ func (c *bundleCache) store(b graph.SymbolBundle) { pkgKey := bundlePackageKey(b.Node.FilePath) c.mu.Lock() defer c.mu.Unlock() + if c.maxBytes <= 0 { + return + } fp, ok := c.fingerprints[pkgKey] if !ok { return } - if len(c.entries) >= bundleCacheMaxEntries { - c.entries = make(map[string]*bundleCacheEntry, bundleCacheMaxEntries) + sz := bundleEntryBytes(b) + if sz > c.maxBytes { + // One entry larger than the entire budget would blow the bound and + // be evicted by the very next insert's wholesale clear anyway. + return + } + if old, ok := c.entries[b.Node.ID]; ok { + // Replacing an existing entry — discount its bytes and drop it so + // curBytes and the count check track the live set. + c.curBytes -= old.bytes + delete(c.entries, b.Node.ID) + } + if len(c.entries) > 0 && (c.curBytes+sz > c.maxBytes || len(c.entries) >= c.maxEntries) { + c.entries = make(map[string]*bundleCacheEntry) + c.curBytes = 0 } - c.entries[b.Node.ID] = &bundleCacheEntry{pkgKey: pkgKey, fp: fp, bundle: b} + c.entries[b.Node.ID] = &bundleCacheEntry{pkgKey: pkgKey, fp: fp, bundle: b, bytes: sz} + c.curBytes += sz } diff --git a/internal/graph/store_sqlite/bundle_cache_test.go b/internal/graph/store_sqlite/bundle_cache_test.go index 6fdf2b2f..f427fad2 100644 --- a/internal/graph/store_sqlite/bundle_cache_test.go +++ b/internal/graph/store_sqlite/bundle_cache_test.go @@ -1,7 +1,9 @@ package store_sqlite import ( + "fmt" "path/filepath" + "sync" "testing" "github.com/zzet/gortex/internal/graph" @@ -11,10 +13,22 @@ func mkFnNode(id, name, file string) *graph.Node { return &graph.Node{ID: id, Kind: graph.KindFunction, Name: name, FilePath: file, Language: "go"} } +// newTestBundleCache builds a cache with the default byte budget without +// consulting the environment, so the fingerprint / invalidation unit tests +// stay hermetic regardless of GORTEX_BUNDLE_CACHE_MAX_MB. +func newTestBundleCache() *bundleCache { + return &bundleCache{ + fingerprints: map[string]uint64{}, + entries: map[string]*bundleCacheEntry{}, + maxBytes: bundleCacheDefaultMaxBytes, + maxEntries: bundleCacheMaxEntries, + } +} + // --- unit tests over the cache logic in isolation --- func TestBundleCache_ServesOnlyValidatedFingerprints(t *testing.T) { - c := &bundleCache{fingerprints: map[string]uint64{}, entries: map[string]*bundleCacheEntry{}} + c := newTestBundleCache() b := graph.SymbolBundle{Node: mkFnNode("pkg/x.go::A", "A", "pkg/x.go")} @@ -34,7 +48,7 @@ func TestBundleCache_ServesOnlyValidatedFingerprints(t *testing.T) { } func TestBundleCache_InvalidatesOnFingerprintChange(t *testing.T) { - c := &bundleCache{fingerprints: map[string]uint64{}, entries: map[string]*bundleCacheEntry{}} + c := newTestBundleCache() c.refresh(map[string]uint64{"pkg": 1}) c.store(graph.SymbolBundle{Node: mkFnNode("pkg/x.go::A", "A", "pkg/x.go")}) @@ -50,7 +64,7 @@ func TestBundleCache_InvalidatesOnFingerprintChange(t *testing.T) { } func TestBundleCache_CrossRepoIsolation(t *testing.T) { - c := &bundleCache{fingerprints: map[string]uint64{}, entries: map[string]*bundleCacheEntry{}} + c := newTestBundleCache() // Two repos with the same inner directory name resolve to DIFFERENT // package keys because the stored file paths are repo-prefixed. c.refresh(map[string]uint64{ @@ -212,3 +226,165 @@ func TestSearchSymbolBundles_UncachedWithoutFingerprints(t *testing.T) { t.Fatalf("uncached path must reflect the new edge live, got %d", len(got.OutEdges)) } } + +// --- byte-budget tests --- + +func TestBundleCache_ByteBudgetEvictionAtBoundary(t *testing.T) { + c := newTestBundleCache() + c.refresh(map[string]uint64{"pkg": 1}) + + // Fixed-width ids so every entry estimates to the same size. + mk := func(i int) graph.SymbolBundle { + return graph.SymbolBundle{Node: mkFnNode(fmt.Sprintf("pkg/x.go::N%03d", i), "W", "pkg/x.go")} + } + unit := bundleEntryBytes(mk(0)) + const k = 4 + c.maxBytes = unit * k // budget holds exactly k entries + + for i := 0; i < k; i++ { + c.store(mk(i)) + } + if len(c.entries) != k { + t.Fatalf("expected %d entries filling the budget, got %d", k, len(c.entries)) + } + if c.curBytes != unit*k { + t.Fatalf("curBytes = %d, want %d", c.curBytes, unit*k) + } + + // One more entry crosses the budget -> wholesale clear, only the newest + // survives and the byte total resets to a single unit. + c.store(mk(k)) + if len(c.entries) != 1 { + t.Fatalf("crossing the budget must clear wholesale to 1 entry, got %d", len(c.entries)) + } + if c.curBytes != unit { + t.Fatalf("curBytes after clear = %d, want %d", c.curBytes, unit) + } + if _, ok := c.lookup(fmt.Sprintf("pkg/x.go::N%03d", k)); !ok { + t.Fatal("the entry that triggered the clear must remain served") + } + if _, ok := c.lookup("pkg/x.go::N000"); ok { + t.Fatal("a pre-clear entry must be gone after the wholesale clear") + } +} + +func TestBundleCache_RefusesEntryLargerThanBudget(t *testing.T) { + c := newTestBundleCache() + c.refresh(map[string]uint64{"pkg": 1}) + b := graph.SymbolBundle{Node: mkFnNode("pkg/x.go::A", "A", "pkg/x.go")} + c.maxBytes = bundleEntryBytes(b) - 1 // budget just below a single entry + + c.store(b) + if _, ok := c.lookup("pkg/x.go::A"); ok { + t.Fatal("an entry larger than the whole budget must not be cached") + } + if len(c.entries) != 0 || c.curBytes != 0 { + t.Fatalf("oversized store must leave the cache empty, got %d entries / %d bytes", + len(c.entries), c.curBytes) + } +} + +func TestBundleCacheMaxBytes_EnvOverride(t *testing.T) { + t.Setenv("GORTEX_BUNDLE_CACHE_MAX_MB", "128") + if got := bundleCacheMaxBytes(); got != 128<<20 { + t.Fatalf("env override = %d, want %d", got, 128<<20) + } + if c := newBundleCache(); c.maxBytes != 128<<20 { + t.Fatalf("newBundleCache maxBytes = %d, want %d", c.maxBytes, 128<<20) + } + + // Empty and unparseable values keep the default. + t.Setenv("GORTEX_BUNDLE_CACHE_MAX_MB", "") + if got := bundleCacheMaxBytes(); got != bundleCacheDefaultMaxBytes { + t.Fatalf("empty override should keep the default, got %d", got) + } + t.Setenv("GORTEX_BUNDLE_CACHE_MAX_MB", "not-a-number") + if got := bundleCacheMaxBytes(); got != bundleCacheDefaultMaxBytes { + t.Fatalf("unparseable override should keep the default, got %d", got) + } +} + +func TestBundleCache_DisabledMode(t *testing.T) { + t.Setenv("GORTEX_BUNDLE_CACHE_MAX_MB", "0") + c := newBundleCache() + if c.maxBytes != 0 { + t.Fatalf("expected a disabled cache (maxBytes 0), got %d", c.maxBytes) + } + c.refresh(map[string]uint64{"pkg": 1}) + c.store(graph.SymbolBundle{Node: mkFnNode("pkg/x.go::A", "A", "pkg/x.go")}) + if len(c.entries) != 0 { + t.Fatalf("a disabled cache must not store, got %d entries", len(c.entries)) + } + if _, ok := c.lookup("pkg/x.go::A"); ok { + t.Fatal("a disabled cache must always miss") + } + + // A negative budget disables too. + t.Setenv("GORTEX_BUNDLE_CACHE_MAX_MB", "-4") + if got := bundleCacheMaxBytes(); got != 0 { + t.Fatalf("a negative override should disable the cache (0), got %d", got) + } +} + +func TestSearchSymbolBundles_DisabledCacheStillServes(t *testing.T) { + t.Setenv("GORTEX_BUNDLE_CACHE_MAX_MB", "0") + s := newBundleTestStore(t) + seedBundleStore(t, s) + s.SetBundleFingerprints(map[string]uint64{"pkg": 1}) + + res, err := s.SearchSymbolBundles("widget", 10) + if err != nil { + t.Fatalf("SearchSymbolBundles with the cache disabled: %v", err) + } + if b, ok := bundleByID(res)["pkg/x.go::A"]; !ok || len(b.OutEdges) != 1 { + t.Fatalf("a disabled cache must still return live bundles, got %+v", b) + } + if s.bundles.maxBytes != 0 { + t.Fatalf("expected the store's cache disabled, got maxBytes %d", s.bundles.maxBytes) + } + if len(s.bundles.entries) != 0 { + t.Fatalf("a disabled cache must stay empty, got %d entries", len(s.bundles.entries)) + } +} + +func TestBundleCache_ConcurrentReadInsert(t *testing.T) { + c := newTestBundleCache() + c.maxBytes = 8 << 10 // small budget so wholesale clears fire under contention + c.refresh(map[string]uint64{"pkg": 1}) + + const workers = 8 + const iters = 3000 + var wg sync.WaitGroup + wg.Add(workers) + for w := 0; w < workers; w++ { + go func(w int) { + defer wg.Done() + for i := 0; i < iters; i++ { + id := fmt.Sprintf("pkg/x.go::N%d_%d", w, i%64) + switch i % 3 { + case 0: + c.store(graph.SymbolBundle{Node: mkFnNode(id, "W", "pkg/x.go")}) + case 1: + _, _ = c.lookup(id) + default: + c.refresh(map[string]uint64{"pkg": uint64(i)}) + } + } + }(w) + } + wg.Wait() + + // No goroutines remain: the running total must exactly equal the summed + // bytes of the surviving entries (the accounting invariant), which also + // proves it never drifted negative under contention. + var sum int64 + for _, e := range c.entries { + sum += e.bytes + } + if c.curBytes != sum { + t.Fatalf("curBytes %d != sum of live entry bytes %d", c.curBytes, sum) + } + if c.curBytes > c.maxBytes { + t.Fatalf("curBytes %d exceeds the byte budget %d", c.curBytes, c.maxBytes) + } +} diff --git a/internal/graph/store_sqlite/store.go b/internal/graph/store_sqlite/store.go index e1316c7d..7c2bb2a0 100644 --- a/internal/graph/store_sqlite/store.go +++ b/internal/graph/store_sqlite/store.go @@ -366,10 +366,7 @@ func openWith(path string, current int, migrations []schemaMigration, allowRebui // and SetBundleFingerprints writes then race only on the cache's // own mutex-guarded maps, not on the Store field. The cache stays // inert (every lookup a miss) until the daemon supplies fingerprints. - s.bundles = &bundleCache{ - fingerprints: map[string]uint64{}, - entries: map[string]*bundleCacheEntry{}, - } + s.bundles = newBundleCache() if err := s.prepare(); err != nil { _ = db.Close() return nil, fmt.Errorf("sqlite prepare: %w", err) diff --git a/internal/indexer/capability_edges.go b/internal/indexer/capability_edges.go index 864843cc..6faf2549 100644 --- a/internal/indexer/capability_edges.go +++ b/internal/indexer/capability_edges.go @@ -233,3 +233,148 @@ func synthesizeCapabilityEdges(g graph.Store) (readsEnv, execProc, fieldAccess i } return readsEnv, execProc, fieldAccess } + +// synthesizeCapabilityEdgesScoped is synthesizeCapabilityEdges restricted to the +// changed repos in an end-of-batch pass. A nil scope runs the whole-graph pass, +// so the fresh-index / single-repo path is byte-identical. +// +// Correctness: every capability edge is FROM the code symbol that performs the +// read/write/call, so a changed repo owns exactly the capability edges its +// reindex dropped; an unchanged repo's are already on disk and are not +// re-derived. The driving scan therefore walks only the changed repos' +// out-edges (GetRepoEdges — one backend query per repo) instead of four +// whole-graph EdgesByKind sweeps. Edge TARGETS may live in any repo (a field or +// env node an unchanged sibling defines), so target identity is never scoped: a +// field target outside the changed repos is confirmed by a cached per-id lookup +// rather than a whole-graph KindField map. Indirect field mutations stay +// whole-graph — their transitive fixpoint lives in indirectMutationEdges (a +// file this pass does not own) and re-affirming an unchanged repo's indirect +// edges is idempotent via the add() dedup. +func synthesizeCapabilityEdgesScoped(g graph.Store, changedPrefixes map[string]bool) (readsEnv, execProc, fieldAccess int) { + if g == nil { + return 0, 0, 0 + } + if changedPrefixes == nil { + return synthesizeCapabilityEdges(g) + } + g.ResolveMutex().Lock() + defer g.ResolveMutex().Unlock() + + type edgeSpec struct { + from, to, origin, file string + line int + kind graph.EdgeKind + meta map[string]any + } + var pending []edgeSpec + seen := map[string]bool{} + add := func(from, to string, kind graph.EdgeKind, origin, file string, line int, meta map[string]any) bool { + key := string(kind) + "\x00" + from + "\x00" + to + if v, _ := meta["via"].(string); v != "" { + key += "\x00" + v + } + if seen[key] { + return false + } + seen[key] = true + pending = append(pending, edgeSpec{from, to, origin, file, line, kind, meta}) + return true + } + + // Field targets: the common case is a symbol writing a field of its own + // (changed) repo, so seed the id set from the changed repos' fields via the + // meta-less light reader. A cross-repo field target is resolved lazily and + // cached, so the pass never materialises the whole-graph KindField map. + fieldIDs := map[string]bool{} + for prefix := range changedPrefixes { + if prefix == "" { + continue + } + for _, n := range repoNodesLightOrFull(g, prefix) { + if n != nil && n.Kind == graph.KindField { + fieldIDs[n.ID] = true + } + } + } + fieldCache := map[string]bool{} + isField := func(id string) bool { + if fieldIDs[id] { + return true + } + if v, ok := fieldCache[id]; ok { + return v + } + n := g.GetNode(id) + f := n != nil && n.Kind == graph.KindField + fieldCache[id] = f + return f + } + + procNodes := map[string]*graph.Node{} + for prefix := range changedPrefixes { + if prefix == "" { + continue + } + for _, e := range g.GetRepoEdges(prefix) { + if e == nil { + continue + } + switch e.Kind { + case graph.EdgeReadsConfig: + if !strings.Contains(e.To, "cfg::env::") { + continue + } + if add(e.From, e.To, graph.EdgeReadsEnv, graph.OriginASTResolved, e.FilePath, e.Line, nil) { + readsEnv++ + } + case graph.EdgeReads: + if !isField(e.To) { + continue + } + if add(e.From, e.To, graph.EdgeAccessesField, graph.OriginASTResolved, e.FilePath, e.Line, map[string]any{"access": "read"}) { + fieldAccess++ + } + case graph.EdgeWrites: + if !isField(e.To) { + continue + } + if add(e.From, e.To, graph.EdgeAccessesField, graph.OriginASTResolved, e.FilePath, e.Line, map[string]any{"access": "write"}) { + fieldAccess++ + } + case graph.EdgeCalls: + mech := processExecMechanism(e.To) + if mech == "" { + continue + } + procID := "string::process::" + mech + if procNodes[procID] == nil { + procNodes[procID] = &graph.Node{ + ID: procID, Kind: graph.KindString, Name: mech, + Meta: map[string]any{"context": "process", "mechanism": mech}, + } + } + if add(e.From, procID, graph.EdgeExecutesProcess, graph.OriginASTInferred, e.FilePath, e.Line, nil) { + execProc++ + } + } + } + } + + for _, s := range indirectMutationEdges(g) { + if add(s.from, s.to, graph.EdgeAccessesField, graph.OriginASTInferred, s.file, s.line, + map[string]any{"access": "write", "indirect": true, "via": s.via}) { + fieldAccess++ + } + } + + for _, n := range procNodes { + g.AddNode(n) + } + for _, s := range pending { + g.AddEdge(&graph.Edge{ + From: s.from, To: s.to, Kind: s.kind, + FilePath: s.file, Line: s.line, Origin: s.origin, Meta: s.meta, + }) + } + return readsEnv, execProc, fieldAccess +} diff --git a/internal/indexer/clone_incremental.go b/internal/indexer/clone_incremental.go index 4f8c4739..6fce556b 100644 --- a/internal/indexer/clone_incremental.go +++ b/internal/indexer/clone_incremental.go @@ -101,15 +101,17 @@ func cloneFuncNodes(nodes []*graph.Node) []*graph.Node { // decodable clone_sig (survivors). This makes Rebuild's CMS/corpus // byte-match what the batch finalise produced. // -// Repo-scoped: it walks AllNodes filtered to n.RepoPrefix == repoPrefix so -// each per-repo index's corpus counts only that repo's bodies — matching -// its repo-scoped LoadCloneShingles seed. An unfiltered AllNodes walk would -// count every repo's bodies into a single repo's corpus and skew its -// threshold. (GetRepoNodes can't be used here: in single-repo / in-memory -// mode repoPrefix is "" and nodes with an empty RepoPrefix are not tracked -// in the byRepo buckets GetRepoNodes reads, so GetRepoNodes("") is always -// empty — the AllNodes+filter form is the one that works for both regimes, -// since "" == "" matches every node.) +// Repo-scoped: it walks the repo's nodes (via cloneRepoNodes) filtered to +// n.RepoPrefix == repoPrefix so each per-repo index's corpus counts only that +// repo's bodies — matching its repo-scoped LoadCloneShingles seed. An +// unfiltered walk would count every repo's bodies into a single repo's corpus +// and skew its threshold. cloneRepoNodes uses GetRepoNodes when repoPrefix is +// non-empty (the daemon multi-repo case), so a warm restart no longer decodes +// the whole graph's nodes to rebuild one repo's index; it falls back to +// AllNodes only in single-repo / in-memory mode, where repoPrefix is "" and +// those nodes are not tracked in the byRepo buckets GetRepoNodes reads (so +// GetRepoNodes("") would be empty and the "" == n.RepoPrefix filter matches +// every node instead). // // Tolerant of a missing/partial sidecar: a body with a clone_sig but no // persisted shingle row still enters the LSH index (so its edges are @@ -134,7 +136,7 @@ func (ci *incrementalCloneIndex) Rebuild(g graph.Store, repoPrefix string) { } } - for _, n := range g.AllNodes() { + for _, n := range cloneRepoNodes(g, repoPrefix) { if n == nil { continue } diff --git a/internal/indexer/clones.go b/internal/indexer/clones.go index f7c48530..8bd5bcea 100644 --- a/internal/indexer/clones.go +++ b/internal/indexer/clones.go @@ -297,12 +297,31 @@ func computeCloneSigFromShingles(cms *clones.CMS, threshold uint32, useFilter bo // in-memory / single-repo store — see incrementalCloneIndex.Rebuild — // so the AllNodes + equality filter is the form that works for both // regimes, since "" == "" matches every node.) +// cloneRepoNodes returns the nodes the per-repo clone passes must walk. In +// daemon multi-repo mode repoPrefix is non-empty, so GetRepoNodes selects just +// that repo's nodes (one backend query, and one meta decode per repo node) +// instead of decoding every node in a many-repo graph only to discard the other +// repos' — the whole-graph AllNodes scan these passes used to run per repo. +// In single-repo / in-memory mode repoPrefix is "" and those nodes are not +// tracked in the per-repo buckets GetRepoNodes reads, so the AllNodes fallback +// (whose "" == n.RepoPrefix filter matches every node) is the only form that +// works there. Callers keep their n.RepoPrefix == repoPrefix guard: a no-op on +// the GetRepoNodes path, load-bearing on the AllNodes fallback. The clone passes +// read blob-only Meta (clone_sig / clone_tokens / clone_shingles), so the full +// GetRepoNodes — not the meta-less light reader — is required here. +func cloneRepoNodes(g graph.Store, repoPrefix string) []*graph.Node { + if repoPrefix != "" { + return g.GetRepoNodes(repoPrefix) + } + return g.AllNodes() +} + func finaliseCloneSignatures(g graph.Store, repoPrefix string) { // First pass: collect every body that has stashed shingles. We // capture the *graph.Node pointers up front so the CMS-build pass // and the signature-compute pass don't both re-walk g.AllNodes(). bodies := make([]*graph.Node, 0, 8192) - for _, n := range g.AllNodes() { + for _, n := range cloneRepoNodes(g, repoPrefix) { if n == nil || n.Meta == nil { continue } @@ -480,7 +499,7 @@ func detectClonesAndEmitEdgesCtx(ctx context.Context, g graph.Store, repoPrefix reporter.Report("clones: gather items", 0, 0) var items []clones.Item - for _, n := range g.AllNodes() { + for _, n := range cloneRepoNodes(g, repoPrefix) { if n == nil || n.Meta == nil { continue } diff --git a/internal/indexer/gc_tune.go b/internal/indexer/gc_tune.go index 62a31a60..e8e628ad 100644 --- a/internal/indexer/gc_tune.go +++ b/internal/indexer/gc_tune.go @@ -6,6 +6,7 @@ import ( "strconv" "strings" "sync" + "time" "go.uber.org/zap" ) @@ -334,11 +335,45 @@ func applyIndexGCTuning(logger *zap.Logger) func() { once.Do(func() { gcTuneMu.Lock() gcTuneDepth-- - if gcTuneDepth == 0 { + closed := gcTuneDepth == 0 + if closed { debug.SetGCPercent(gcTunePrevPct) debug.SetMemoryLimit(gcTunePrevLim) } gcTuneMu.Unlock() + // Scavenge the cold-index burst's heap high-water once the + // window is fully closed — outside the lock, since FreeOSMemory + // is a full GC cycle that must not serialise sibling index + // calls waiting on gcTuneMu. + if closed { + freeOSMemoryAfterColdIndex(logger) + } }) } } + +// memReleaseEnabled reports whether post-burst heap release is active. On by +// default; GORTEX_DAEMON_MEMRELEASE=0 (or "false") disables it. The check is +// duplicated here (rather than shared) because the canonical release helper +// lives in the cmd layer, which this package must not import. +func memReleaseEnabled() bool { + v := os.Getenv("GORTEX_DAEMON_MEMRELEASE") + return v != "0" && !strings.EqualFold(v, "false") +} + +// freeOSMemoryAfterColdIndex returns the cold-index burst's heap high-water +// to the OS once the tuning window has fully closed. A cold index churns +// multi-GB of parse / node / edge allocation; debug.FreeOSMemory forces a GC +// + scavenge so that peak does not stay resident on the daemon's footprint +// until some later collection happens to reclaim it. +func freeOSMemoryAfterColdIndex(logger *zap.Logger) { + if !memReleaseEnabled() { + return + } + start := time.Now() + debug.FreeOSMemory() + if logger != nil { + logger.Debug("indexer: released heap to OS after cold index", + zap.Duration("elapsed", time.Since(start))) + } +} diff --git a/internal/indexer/gc_tune_test.go b/internal/indexer/gc_tune_test.go index 2d012661..b5e49a6b 100644 --- a/internal/indexer/gc_tune_test.go +++ b/internal/indexer/gc_tune_test.go @@ -3,6 +3,7 @@ package indexer import ( "errors" "os" + "runtime" "runtime/debug" "testing" ) @@ -502,3 +503,52 @@ func TestCPUClampEnabled(t *testing.T) { }) } } + +func TestMemReleaseEnabled(t *testing.T) { + tests := []struct { + val string + want bool + }{ + {"", true}, // unset => on by default + {"1", true}, + {"true", true}, + {"anything", true}, + {"0", false}, // kill-switch + {"false", false}, + {"FALSE", false}, + } + for _, tt := range tests { + t.Run(tt.val, func(t *testing.T) { + t.Setenv("GORTEX_DAEMON_MEMRELEASE", tt.val) + if got := memReleaseEnabled(); got != tt.want { + t.Fatalf("memReleaseEnabled() with %q = %v, want %v", tt.val, got, tt.want) + } + }) + } +} + +func TestFreeOSMemoryAfterColdIndex(t *testing.T) { + // Kill-switch: a no-op that must not panic (logger is nil-safe). + t.Setenv("GORTEX_DAEMON_MEMRELEASE", "0") + freeOSMemoryAfterColdIndex(nil) + + // Enabled: forces a scavenge; HeapReleased must not go backwards. + t.Setenv("GORTEX_DAEMON_MEMRELEASE", "1") + sink := make([]byte, 8<<20) + for i := range sink { + sink[i] = byte(i) + } + // Force the writes to land (so the allocation is not elided), then let the + // reference drop here: sink is dead past this point, so the scavenge below + // has heap to return. + runtime.KeepAlive(sink) + + var before, after runtime.MemStats + runtime.ReadMemStats(&before) + freeOSMemoryAfterColdIndex(nil) + runtime.ReadMemStats(&after) + if after.HeapReleased < before.HeapReleased { + t.Fatalf("HeapReleased went backwards: before=%d after=%d", + before.HeapReleased, after.HeapReleased) + } +} diff --git a/internal/indexer/multi.go b/internal/indexer/multi.go index 54da5005..ce245d79 100644 --- a/internal/indexer/multi.go +++ b/internal/indexer/multi.go @@ -840,24 +840,79 @@ func (mi *MultiIndexer) RunGlobalGraphPasses(ctx context.Context) { // low-yield pass left no breadcrumb — the source of the multi-minute // "silent" span after resolve on a cold index. globalStart := time.Now() + + // Acquire the changed-repo scope once for the whole run and derive the two + // shapes the passes below consume. A nil scope means whole-graph — the + // fresh-index / one-off behaviour every pass keeps as its fallback — so an + // unscoped run is byte-identical to before. A non-nil scope (armed by the + // daemon warmup / hourly janitor via ArmBatchScope) narrows each pass to the + // repos that re-indexed this batch: an unchanged repo's derived edges are + // already on disk and were never dropped, so re-deriving them is skipped. + // - changedPrefixes: the changed-repo prefix set, for the edge-driven passes + // (capability, test edges, framework synthesis, external calls), each of + // which owns an edge iff its FROM node is in a changed repo. + // - scopedTypeIfaceIDs: the changed repos' type/interface node IDs, for the + // implements/overrides inference. The scoped inference keeps a pair when + // EITHER endpoint is in this set, so a cross-repo override whose child is + // in a changed repo and whose parent is in an unchanged one (or vice + // versa) is still re-derived; structural implements never crosses repos + // (its same-repo gate), so scoping both its sides here is complete. + scope := mi.takeBatchScope() + var changedPrefixes map[string]bool + var scopedTypeIfaceIDs map[string]bool + if scope != nil { + changedPrefixes = make(map[string]bool, len(scope)) + scopedTypeIfaceIDs = map[string]bool{} + for prefix := range scope { + changedPrefixes[prefix] = true + if prefix == "" { + continue + } + for _, n := range repoNodesLightOrFull(mi.graph, prefix) { + if n == nil { + continue + } + if n.Kind == graph.KindType || n.Kind == graph.KindInterface { + scopedTypeIfaceIDs[n.ID] = true + } + } + } + } + implStart := time.Now() - implAdded := r.InferImplements() + implAdded := 0 + switch { + case scope == nil: + implAdded = r.InferImplements() + case len(scopedTypeIfaceIDs) > 0: + // Empty set => no type/interface changed in the batch => no inferred + // implements edge could have been dropped, so the pass is skipped. + implAdded = r.InferImplementsScoped(scopedTypeIfaceIDs, scopedTypeIfaceIDs) + } mi.logger.Info("global pass: infer implements", zap.Int("added", implAdded), + zap.Bool("scoped", scope != nil), zap.Duration("elapsed", time.Since(implStart))) overStart := time.Now() - overAdded := r.InferOverrides() + overAdded := 0 + switch { + case scope == nil: + overAdded = r.InferOverrides() + case len(scopedTypeIfaceIDs) > 0: + overAdded = r.InferOverridesScoped(scopedTypeIfaceIDs) + } mi.logger.Info("global pass: infer overrides", zap.Int("added", overAdded), + zap.Bool("scoped", scope != nil), zap.Duration("elapsed", time.Since(overStart))) testStart := time.Now() - marked, emitted := markTestSymbolsAndEmitEdges(mi.graph) + marked, emitted := markTestSymbolsAndEmitEdgesScoped(mi.graph, changedPrefixes) mi.logger.Info("global pass: test edges", zap.Int("test_symbols", marked), zap.Int("edges", emitted), zap.Duration("elapsed", time.Since(testStart))) capStart := time.Now() - capRe, capEp, capFa := synthesizeCapabilityEdges(mi.graph) + capRe, capEp, capFa := synthesizeCapabilityEdgesScoped(mi.graph, changedPrefixes) mi.logger.Info("global pass: capability edges", zap.Int("reads_env", capRe), zap.Int("executes_process", capEp), @@ -881,8 +936,10 @@ func (mi *MultiIndexer) RunGlobalGraphPasses(ctx context.Context) { // its incremental clone index is reseeded lazily on its first later edit // (indexFile: if !built → Rebuild), so skipping its full-graph detect + // Rebuild here is sound and cuts an N-repo warm restart from N full-graph - // clone walks to just the changed repos'. A nil scope runs every repo. - scope := mi.takeBatchScope() + // clone walks to just the changed repos'. A nil scope runs every repo. The + // scope is taken once at the top of RunGlobalGraphPasses and shared by every + // pass; the clone passes reuse it here (takeBatchScope must not be called + // twice — it clears the armed scope on the first read). inCloneScope := func(prefix string) bool { if scope == nil { return true @@ -950,7 +1007,7 @@ func (mi *MultiIndexer) RunGlobalGraphPasses(ctx context.Context) { // edge. reporter.Report("framework dispatch synthesis (global)", 0, 0) fwStart := time.Now() - fwRep := resolver.RunFrameworkSynthesizers(mi.graph) + fwRep := resolver.RunFrameworkSynthesizersScoped(mi.graph, changedPrefixes) mi.logger.Info("global pass: framework dispatch synthesis", zap.Int("edges", fwRep.Total), zap.Any("per_synthesizer", fwRep.Per), @@ -963,9 +1020,16 @@ func (mi *MultiIndexer) RunGlobalGraphPasses(ctx context.Context) { // left to materialise into call-chain terminals. reporter.Report("external-call synthesis (global)", 0, 0) extStart := time.Now() - extCalls := resolver.SynthesizeExternalCalls(mi.graph, mi.externalCallSynthesisEnabled()) + extEnabled := mi.externalCallSynthesisEnabled() + extCalls := 0 + if scope != nil { + extCalls = resolver.SynthesizeExternalCallsForRepos(mi.graph, extEnabled, changedPrefixes) + } else { + extCalls = resolver.SynthesizeExternalCalls(mi.graph, extEnabled) + } mi.logger.Info("global pass: external-call synthesis", zap.Int("edges", extCalls), + zap.Bool("scoped", scope != nil), zap.Duration("elapsed", time.Since(extStart))) // Cross-repo edge layer. Runs after InferImplements / InferOverrides // so the implements / extends edges they materialise across repo @@ -980,6 +1044,21 @@ func (mi *MultiIndexer) RunGlobalGraphPasses(ctx context.Context) { zap.Duration("total", time.Since(globalStart))) } +// repoNodesLightOrFull returns a repo's nodes for read-only structural +// inspection (id / kind / repo prefix), preferring the meta-less +// LightNodeReader fast path when the backend implements it so the enriched meta +// blob a scope build never reads stays server-side. The returned nodes MUST NOT +// be written back through AddNode/AddBatch — the light projection drops any +// non-promoted meta — which holds here because the only caller reads struct +// fields to build an ID set and discards the nodes. Falls back to the full +// GetRepoNodes when the backend (e.g. in-memory) has no separate blob to skip. +func repoNodesLightOrFull(g graph.Store, prefix string) []*graph.Node { + if lr, ok := g.(graph.LightNodeReader); ok { + return lr.GetRepoNodesLight(prefix) + } + return g.GetRepoNodes(prefix) +} + // externalCallSynthesisEnabled resolves whether external-call placeholder // synthesis should run over the shared graph. The pass is graph-wide, so // it is enabled when any tracked repo opted in — a repo that wants the diff --git a/internal/indexer/scoped_global_passes_test.go b/internal/indexer/scoped_global_passes_test.go new file mode 100644 index 00000000..f9036d60 --- /dev/null +++ b/internal/indexer/scoped_global_passes_test.go @@ -0,0 +1,173 @@ +package indexer + +import ( + "context" + "iter" + "testing" + + "github.com/zzet/gortex/internal/graph" +) + +// idxCountingStore wraps a graph.Store and records which node-read paths a pass +// takes, so a test can prove a repo-scoped pass drives off GetRepoNodes / +// GetRepoEdges for the changed repo and never materialises another repo's nodes +// via a whole-graph AllNodes / NodesByKind scan. +type idxCountingStore struct { + graph.Store + allNodes int + repoNodes map[string]int + repoEdges map[string]int + nodesReturned int +} + +func newIdxCountingStore(s graph.Store) *idxCountingStore { + return &idxCountingStore{Store: s, repoNodes: map[string]int{}, repoEdges: map[string]int{}} +} + +func (c *idxCountingStore) AllNodes() []*graph.Node { + c.allNodes++ + ns := c.Store.AllNodes() + c.nodesReturned += len(ns) + return ns +} + +func (c *idxCountingStore) GetRepoNodes(prefix string) []*graph.Node { + c.repoNodes[prefix]++ + ns := c.Store.GetRepoNodes(prefix) + c.nodesReturned += len(ns) + return ns +} + +func (c *idxCountingStore) GetRepoEdges(prefix string) []*graph.Edge { + c.repoEdges[prefix]++ + return c.Store.GetRepoEdges(prefix) +} + +func (c *idxCountingStore) GetFileNodes(path string) []*graph.Node { + ns := c.Store.GetFileNodes(path) + c.nodesReturned += len(ns) + return ns +} + +func (c *idxCountingStore) NodesByKind(k graph.NodeKind) iter.Seq[*graph.Node] { + inner := c.Store.NodesByKind(k) + return func(yield func(*graph.Node) bool) { + for n := range inner { + c.nodesReturned++ + if !yield(n) { + return + } + } + } +} + +// twoRepoFuncGraph builds a graph with a handful of function/method nodes in +// each of two repos, so the per-repo readers have something to return. +func twoRepoFuncGraph() *graph.Graph { + g := graph.New() + for _, spec := range []struct{ repo, file string }{{"repoA", "a.go"}, {"repoB", "b.go"}} { + g.AddNode(&graph.Node{ID: spec.repo + "::" + spec.file + "::Fn", Kind: graph.KindFunction, Name: "Fn", RepoPrefix: spec.repo, FilePath: spec.file}) + g.AddNode(&graph.Node{ID: spec.repo + "::" + spec.file + "::Fn2", Kind: graph.KindFunction, Name: "Fn2", RepoPrefix: spec.repo, FilePath: spec.file}) + } + return g +} + +// TestCloneRepoNodes_ScopedNeverMaterialisesOtherRepo asserts the clone detect +// and incremental Rebuild passes for one repo read only that repo's nodes via +// GetRepoNodes — never the whole-graph AllNodes scan, and never the sibling +// repo's node bucket. +func TestCloneRepoNodes_ScopedNeverMaterialisesOtherRepo(t *testing.T) { + cs := newIdxCountingStore(twoRepoFuncGraph()) + + // Detect for repoA: finalise + detect both walk cloneRepoNodes(repoA). + detectClonesAndEmitEdgesCtx(context.Background(), cs, "repoA", 0.8) + // Incremental index rebuild for repoA reseeds from the same repo's nodes. + ci := newIncrementalCloneIndex() + ci.Rebuild(cs, "repoA") + + if cs.allNodes != 0 { + t.Errorf("clone passes for repoA must not call AllNodes(); got %d calls", cs.allNodes) + } + if cs.repoNodes["repoA"] == 0 { + t.Errorf("clone passes for repoA must read via GetRepoNodes(\"repoA\")") + } + if cs.repoNodes["repoB"] != 0 { + t.Errorf("clone passes for repoA must never materialise repoB's nodes; got %d GetRepoNodes(\"repoB\") calls", cs.repoNodes["repoB"]) + } +} + +// TestCloneRepoNodes_EmptyPrefixFallsBackToAllNodes asserts the single-repo / +// in-memory regime (empty prefix, nodes not tracked in the byRepo buckets) still +// uses the AllNodes fallback — GetRepoNodes("") would be empty. +func TestCloneRepoNodes_EmptyPrefixFallsBackToAllNodes(t *testing.T) { + cs := newIdxCountingStore(twoRepoFuncGraph()) + detectClonesAndEmitEdgesCtx(context.Background(), cs, "", 0.8) + if cs.allNodes == 0 { + t.Errorf("empty-prefix clone detect must fall back to AllNodes()") + } +} + +func accessesFieldFrom(g graph.Store, repoPrefix string) map[string]bool { + out := map[string]bool{} + for _, e := range g.AllEdges() { + if e.Kind != graph.EdgeAccessesField { + continue + } + n := g.GetNode(e.From) + if n != nil && n.RepoPrefix == repoPrefix { + out[e.From+"->"+e.To] = true + } + } + return out +} + +// capabilityFixture builds a field write in repo A (the changed repo) plus a +// large field population in repo B, so the whole-graph fieldIDs scan the +// unscoped capability pass runs materialises far more nodes than the scoped pass +// that reads only repo A's nodes. +func capabilityFixture() *graph.Graph { + g := graph.New() + g.AddNode(&graph.Node{ID: "repoA::a.go::Foo", Kind: graph.KindType, Name: "Foo", RepoPrefix: "repoA", FilePath: "a.go"}) + g.AddNode(&graph.Node{ID: "repoA::a.go::Foo.set", Kind: graph.KindMethod, Name: "set", RepoPrefix: "repoA", FilePath: "a.go", Meta: map[string]any{"receiver": "Foo"}}) + g.AddNode(&graph.Node{ID: "repoA::a.go::Foo.count", Kind: graph.KindField, Name: "count", RepoPrefix: "repoA", FilePath: "a.go", Meta: map[string]any{"receiver": "Foo"}}) + g.AddEdge(&graph.Edge{From: "repoA::a.go::Foo.set", To: "repoA::a.go::Foo.count", Kind: graph.EdgeWrites, FilePath: "a.go"}) + // Repo B: a large, unchanged field population the scoped pass must not scan. + for i := 0; i < 80; i++ { + id := "repoB::b.go::F" + string(rune('A'+i%26)) + string(rune('0'+i/26)) + g.AddNode(&graph.Node{ID: id, Kind: graph.KindField, Name: "f", RepoPrefix: "repoB", FilePath: "b.go", Meta: map[string]any{"receiver": "Bar"}}) + } + return g +} + +// TestSynthesizeCapabilityEdgesScoped_ParityAndFewerReads asserts the scoped +// capability pass emits the same accesses_field edge for the changed repo as the +// unscoped pass, drives its sweep off GetRepoEdges, and materialises fewer nodes +// (it never runs the whole-graph KindField scan that seeds fieldIDs). +func TestSynthesizeCapabilityEdgesScoped_ParityAndFewerReads(t *testing.T) { + full := newIdxCountingStore(capabilityFixture()) + synthesizeCapabilityEdges(full) + wantA := accessesFieldFrom(full, "repoA") + if !wantA["repoA::a.go::Foo.set->repoA::a.go::Foo.count"] { + t.Fatalf("unscoped pass did not emit repo A's accesses_field edge: %v", wantA) + } + + scoped := newIdxCountingStore(capabilityFixture()) + synthesizeCapabilityEdgesScoped(scoped, map[string]bool{"repoA": true}) + gotA := accessesFieldFrom(scoped, "repoA") + if len(gotA) != len(wantA) { + t.Fatalf("scoped capability repo-A edges = %v, want %v", gotA, wantA) + } + for k := range wantA { + if !gotA[k] { + t.Errorf("scoped run dropped repo A's capability edge %q", k) + } + } + + if scoped.repoEdges["repoA"] == 0 { + t.Errorf("scoped capability must drive its sweep off GetRepoEdges(\"repoA\")") + } + if scoped.nodesReturned >= full.nodesReturned { + t.Errorf("scoped capability should materialise fewer nodes than unscoped: scoped=%d full=%d", + scoped.nodesReturned, full.nodesReturned) + } +} diff --git a/internal/indexer/test_edges.go b/internal/indexer/test_edges.go index 69b1654d..e10d5812 100644 --- a/internal/indexer/test_edges.go +++ b/internal/indexer/test_edges.go @@ -33,6 +33,20 @@ import ( // Returns counts for telemetry: number of nodes marked as test, // number of EdgeTests emitted. func markTestSymbolsAndEmitEdges(g graph.Store) (markedTests int, edgesEmitted int) { + return markTestSymbolsAndEmitEdgesScoped(g, nil) +} + +// markTestSymbolsAndEmitEdgesScoped is markTestSymbolsAndEmitEdges with an armed +// changed-repo scope for the end-of-batch pass. A nil scope emits over the whole +// graph, so the fresh-index / single-repo path is byte-identical. +// +// Pass 1 (test-symbol classification) always runs whole-graph: the testNodes +// membership set it builds must be COMPLETE, because Pass 2 skips test→test +// calls via testNodes[e.To] and a callee can be a test in an unchanged repo +// (a cross-repo test→test call). Only Pass 2's driving EdgeCalls scan is scoped +// — an EdgeTests edge is FROM a test function, so a changed repo owns exactly +// the test edges its reindex dropped; an unchanged repo's persist on disk. +func markTestSymbolsAndEmitEdgesScoped(g graph.Store, changedPrefixes map[string]bool) (markedTests int, edgesEmitted int) { if g == nil { return 0, 0 } @@ -44,6 +58,19 @@ func markTestSymbolsAndEmitEdges(g graph.Store) (markedTests int, edgesEmitted i g.ResolveMutex().Lock() defer g.ResolveMutex().Unlock() + testNodes, markedTests := markTestSymbolsLocked(g) + if len(testNodes) == 0 { + return markedTests, 0 + } + edgesEmitted = emitTestEdgesLocked(g, testNodes, changedPrefixes) + return markedTests, edgesEmitted +} + +// markTestSymbolsLocked runs Pass 1: it stamps test Meta on every test symbol +// and returns the complete test-node membership set plus the marked count. The +// caller must hold g.ResolveMutex(). Always whole-graph — see the scoped +// entry point for why the set must be complete. +func markTestSymbolsLocked(g graph.Store) (testNodes map[string]bool, markedTests int) { // Pass 1: classify file nodes, then function/method nodes. Build // a local testNodes set keyed by node id so Pass 2 can probe it // without re-walking the Meta. (Node.Meta mutations on returned @@ -101,7 +128,7 @@ func markTestSymbolsAndEmitEdges(g graph.Store) (markedTests int, edgesEmitted i } } - testNodes := map[string]bool{} + testNodes = map[string]bool{} stampTestSymbol := func(n *graph.Node) { inTestFile := testFiles[n.FilePath] var role, runner string @@ -147,53 +174,71 @@ func markTestSymbolsAndEmitEdges(g graph.Store) (markedTests int, edgesEmitted i stampTestSymbol(n) } } + return testNodes, markedTests +} - // Pass 2: walk EdgeCalls; for each (test, non-test) pair, emit a - // parallel EdgeTests. We dedupe per (From, To) because a single - // test can call the same subject multiple times. The testNodes set - // built in Pass 1 is the authoritative source — no inline GetNode - // is needed because the From / To kind filter is already enforced - // by "From must be a test symbol" (only function/method ids land - // in testNodes). +// emitTestEdgesLocked runs Pass 2: for each (test, non-test) call it emits a +// parallel EdgeTests, deduped per (From, To) because a single test can call the +// same subject repeatedly. The testNodes set from Pass 1 is authoritative — no +// inline GetNode is needed because "From must be a test symbol" already enforces +// the kind filter (only function/method ids land in testNodes). The caller must +// hold g.ResolveMutex(). +// +// With a nil scope it walks every EdgeCalls edge; with a scope it walks only the +// changed repos' out-edges (GetRepoEdges — one backend query per repo). The +// testNodes[e.To] test→test skip stays correct across repos because testNodes is +// complete (Pass 1 is whole-graph). +func emitTestEdgesLocked(g graph.Store, testNodes map[string]bool, changedPrefixes map[string]bool) int { + edgesEmitted := 0 seen := map[string]bool{} - type pair struct{ from, to string } - var pending []struct { - pair pair - edge *graph.Edge + type pending struct { + from, to, file string + line int } - for e := range g.EdgesByKind(graph.EdgeCalls) { - if e == nil { - continue + var out []pending + process := func(e *graph.Edge) { + if e == nil || e.Kind != graph.EdgeCalls { + return } if !testNodes[e.From] { - continue + return } if testNodes[e.To] { - continue // test → test calls are infrastructure, not subject coverage + return // test → test calls are infrastructure, not subject coverage } key := e.From + "\x00" + e.To if seen[key] { - continue + return } seen[key] = true - pending = append(pending, struct { - pair pair - edge *graph.Edge - }{pair{e.From, e.To}, e}) + out = append(out, pending{from: e.From, to: e.To, file: e.FilePath, line: e.Line}) } - for _, p := range pending { - newEdge := &graph.Edge{ - From: p.pair.from, - To: p.pair.to, + if changedPrefixes == nil { + for e := range g.EdgesByKind(graph.EdgeCalls) { + process(e) + } + } else { + for prefix := range changedPrefixes { + if prefix == "" { + continue + } + for _, e := range g.GetRepoEdges(prefix) { + process(e) + } + } + } + for _, p := range out { + g.AddEdge(&graph.Edge{ + From: p.from, + To: p.to, Kind: graph.EdgeTests, - FilePath: p.edge.FilePath, - Line: p.edge.Line, + FilePath: p.file, + Line: p.line, Origin: graph.OriginASTInferred, - } - g.AddEdge(newEdge) + }) edgesEmitted++ } - return markedTests, edgesEmitted + return edgesEmitted } // detectTestRunnerForFile resolves the runner identifier for a test file @@ -213,10 +258,10 @@ func markTestSymbolsAndEmitEdges(g graph.Store) (markedTests int, edgesEmitted i // 3. Language-level defaults that hold regardless of imports: // - Go always uses `gotest` — `go test` is the only runner. // - Python defaults to `pytest` (auto-discovery picks up unittest -// test cases too; rare files that import only `unittest` are -// caught by step 2). +// test cases too; rare files that import only `unittest` are +// caught by step 2). // - Ruby falls back to `rspec` for `_spec.rb` and `minitest` for -// `_test.rb`. +// `_test.rb`. // // Returns "" when no signal applies; the caller leaves test_runner // unset rather than guessing. diff --git a/internal/indexer/watcher.go b/internal/indexer/watcher.go index ded12784..4acda9d2 100644 --- a/internal/indexer/watcher.go +++ b/internal/indexer/watcher.go @@ -938,6 +938,10 @@ func (w *Watcher) patchGraphNoResolve(path string, kind ChangeKind) { } case ChangeDeleted, ChangeRenamed: w.indexer.EvictFile(path) + // Keep the persisted mtime in step with the eviction (see + // forgetDeletedFileMtime) so a warm restart after a storm-drain + // delete does not treat the vanished path as a phantom deletion. + w.forgetDeletedFileMtime(w.indexer.RelKey(path)) } } @@ -970,6 +974,26 @@ func (w *Watcher) reconcileKindWithDisk(path string, kind ChangeKind) ChangeKind return kind } +// forgetDeletedFileMtime drops a just-evicted file's recorded mtime from +// both the in-memory map and the store's FileMtime sidecar. EvictFile +// removes the file's nodes but leaves its mtime behind, so without this the +// persisted mtime row outlives the file: the next warm restart reads it +// back, finds the path gone from disk, and treats it as a phantom deletion +// — re-running a scoped reconcile for a file that is already correct on +// every boot. Mirrors IncrementalReindex's deletion handling: prune the +// in-memory map first (pruneDeletedFileMtimes documents that its caller has +// already done so, and a later snapshot persist would otherwise resurrect +// the row from the stale in-memory entry), then the store, which self-skips +// on a backend without the FileMtimeDeleter capability. relPath must be the +// canonical relKey the mtime map and store are keyed on — the same key +// EvictFile evicted the file's nodes under. +func (w *Watcher) forgetDeletedFileMtime(relPath string) { + w.indexer.mtimeMu.Lock() + delete(w.indexer.fileMtimes, relPath) + w.indexer.mtimeMu.Unlock() + w.indexer.pruneDeletedFileMtimes([]string{relPath}) +} + func (w *Watcher) patchGraph(path string, kind ChangeKind) { w.patchMu.Lock() defer w.patchMu.Unlock() @@ -1085,6 +1109,13 @@ func (w *Watcher) patchGraph(path string, kind ChangeKind) { nodesRemoved = nr edgesRemoved = er + // The file is genuinely gone from disk here — reconcileKindWithDisk + // already downgraded a replace/revert (path still present) to + // ChangeModified. Drop its now-orphaned mtime so a warm restart does + // not re-discover the vanished path as a phantom deletion. relPath is + // the canonical relKey EvictFile evicted under. + w.forgetDeletedFileMtime(relPath) + // Notify callback: old symbols removed, no new symbols. w.symbolChangeCbMu.RLock() cb := w.symbolChangeCb diff --git a/internal/indexer/watcher_delete_mtime_test.go b/internal/indexer/watcher_delete_mtime_test.go new file mode 100644 index 00000000..47e0a276 --- /dev/null +++ b/internal/indexer/watcher_delete_mtime_test.go @@ -0,0 +1,157 @@ +package indexer + +import ( + "context" + "os" + "path/filepath" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "go.uber.org/zap" + + "github.com/zzet/gortex/internal/config" + "github.com/zzet/gortex/internal/graph" + "github.com/zzet/gortex/internal/graph/store_sqlite" +) + +// newSqliteWatcher builds a repo of the given files, full-indexes it into a +// fresh sqlite store (so the per-file mtime rows a warm restart reads are +// actually persisted), and returns a Watcher wired to it ready to drive +// patchGraph / patchGraphNoResolve directly. The sqlite store implements the +// FileMtime{Writer,Reader,Deleter} capabilities the watcher's patch paths key +// off, so the in-memory graph backend's no-op behavior is covered separately. +func newSqliteWatcher(t *testing.T, files map[string]string) (dir string, idx *Indexer, w *Watcher, s *store_sqlite.Store) { + t.Helper() + dir = t.TempDir() + for name, content := range files { + writeTestFile(t, filepath.Join(dir, name), content) + } + s = openTestSqlite(t) + idx = New(graph.Store(s), newTestRegistry(), config.Default().Index, zap.NewNop()) + idx.SetRootPath(dir) + _, err := idx.IndexCtx(context.Background(), dir) + require.NoError(t, err) + w, err = NewWatcher(idx, config.WatchConfig{Enabled: true, DebounceMs: 10}, zap.NewNop()) + require.NoError(t, err) + return dir, idx, w, s +} + +// TestWatcher_DeletePatchPrunesPersistedMtime is the core regression: when the +// live watcher patches a deleted file it must drop that file's persisted mtime +// row. Before the fix EvictFile removed the file's nodes but left its mtime +// behind, so the next warm restart read the orphaned row back, found the path +// gone from disk, and treated it as a phantom deletion — forcing a scoped +// reconcile for an already-correct repo on every boot. +func TestWatcher_DeletePatchPrunesPersistedMtime(t *testing.T) { + dir, idx, w, s := newSqliteWatcher(t, map[string]string{ + "gone.go": "package main\n\nfunc Gone() {}\n", + "keep.go": "package main\n\nfunc Keep() {}\n", + }) + + before := s.LoadFileMtimes("") + require.Contains(t, before, "gone.go", "the full index must persist gone.go's mtime") + require.Contains(t, before, "keep.go", "the full index must persist keep.go's mtime") + + gonePath := filepath.Join(dir, "gone.go") + require.NoError(t, os.Remove(gonePath)) + w.patchGraph(gonePath, ChangeDeleted) + + after := s.LoadFileMtimes("") + assert.NotContains(t, after, "gone.go", + "a delete patch must prune the vanished file's persisted mtime row") + assert.Contains(t, after, "keep.go", + "a delete patch must not touch a sibling file's mtime row") + + // The in-memory map must be pruned too, or a later snapshot persist would + // resurrect the store row from the stale in-memory entry. + assert.NotContains(t, idx.FileMtimes(), "gone.go", + "a delete patch must also drop the in-memory mtime entry") +} + +// TestWatcher_StormDrainDeletePrunesPersistedMtime covers the batched delete +// path: storm-drain re-indexing routes deletes through patchGraphNoResolve, +// which must prune the persisted mtime just like the debounced patchGraph does. +func TestWatcher_StormDrainDeletePrunesPersistedMtime(t *testing.T) { + dir, idx, w, s := newSqliteWatcher(t, map[string]string{ + "batch.go": "package main\n\nfunc Batch() {}\n", + }) + require.Contains(t, s.LoadFileMtimes(""), "batch.go") + + path := filepath.Join(dir, "batch.go") + require.NoError(t, os.Remove(path)) + w.patchGraphNoResolve(path, ChangeDeleted) + + assert.NotContains(t, s.LoadFileMtimes(""), "batch.go", + "the storm-drain delete path must prune the persisted mtime too") + assert.NotContains(t, idx.FileMtimes(), "batch.go", + "the storm-drain delete path must drop the in-memory mtime entry") +} + +// TestWatcher_ModifyPatchPersistsMtimeToStore is the other half of the +// contract: a modify patch must persist the file's advanced mtime so a warm +// restart does not re-detect the already-patched file as changed. A structural +// modify reindexes through IndexFile, whose recordFileMtime does the persist. +func TestWatcher_ModifyPatchPersistsMtimeToStore(t *testing.T) { + dir, _, w, s := newSqliteWatcher(t, map[string]string{ + "main.go": "package main\n\nfunc First() {}\n", + }) + path := filepath.Join(dir, "main.go") + + before := s.LoadFileMtimes("")["main.go"] + require.NotZero(t, before) + + // Stamp a strictly later mtime so the advance is observable, and add a + // function so the change is structural (forces the reindex path). + future := time.Now().Add(2 * time.Second) + writeTestFile(t, path, "package main\n\nfunc First() {}\n\nfunc Second() {}\n") + require.NoError(t, os.Chtimes(path, future, future)) + w.patchGraph(path, ChangeModified) + + info, statErr := os.Stat(path) + require.NoError(t, statErr) + after := s.LoadFileMtimes("") + assert.Equal(t, info.ModTime().UnixNano(), after["main.go"], + "a modify patch must persist the file's current on-disk mtime to the store") + assert.Greater(t, after["main.go"], before, + "the persisted mtime must advance past the modify") +} + +// TestWatcher_DeleteEventForPresentFileKeepsMtime guards the interaction with +// reconcileKindWithDisk: a stale delete/rename event whose path is still a +// regular file is a replace/revert, downgraded to a modify — so the mtime must +// be refreshed, never pruned. This proves the prune fires only on a genuine +// on-disk deletion. +func TestWatcher_DeleteEventForPresentFileKeepsMtime(t *testing.T) { + dir, _, w, s := newSqliteWatcher(t, map[string]string{ + "revert.go": "package main\n\nfunc Revert() {}\n", + }) + path := filepath.Join(dir, "revert.go") + require.Contains(t, s.LoadFileMtimes(""), "revert.go") + + w.patchGraph(path, ChangeDeleted) + + assert.Contains(t, s.LoadFileMtimes(""), "revert.go", + "a delete event for a still-present file (a revert) must not prune its mtime") +} + +// TestWatcher_DeletePatchInMemoryBackendSkipsStore proves the store prune +// self-skips cleanly on a backend that implements no FileMtimeDeleter (the +// in-memory graph): no panic, no capability required, and the in-memory mtime +// entry is still pruned. Injecting a store whose DeleteFileMtimes returns an +// error is not done here — it would require a full fake graph.Store (AddBatch / +// GetFileNodes / EvictFile / ...) for a path whose only effect is a logged, +// non-fatal warn — so the realistic "store cannot persist" branch is the +// capability-absent one exercised below. +func TestWatcher_DeletePatchInMemoryBackendSkipsStore(t *testing.T) { + dir, idx, w := inertTestWatcher(t, "solo.go", "package main\n\nfunc Solo() {}\n") + path := filepath.Join(dir, "solo.go") + require.Contains(t, idx.FileMtimes(), "solo.go") + + require.NoError(t, os.Remove(path)) + w.patchGraph(path, ChangeDeleted) + + assert.NotContains(t, idx.FileMtimes(), "solo.go", + "the in-memory mtime entry must be pruned even without a store deleter") +} diff --git a/internal/mcp/server.go b/internal/mcp/server.go index a47c97d8..0a624ac2 100644 --- a/internal/mcp/server.go +++ b/internal/mcp/server.go @@ -6,6 +6,7 @@ import ( "fmt" "math" "os" + "runtime/debug" "sort" "strings" "sync" @@ -2249,6 +2250,30 @@ func (s *Server) RunAnalysis() { if s.graphInvalidatedBroadcaster != nil && s.graph != nil { s.graphInvalidatedBroadcaster.broadcast(s.graph.NodeCount(), s.graph.EdgeCount(), "reanalysis") } + + // A full analysis pass (PageRank / Leiden / HITS / hotspots over the + // whole graph) is one of the daemon's largest on-demand allocation + // bursts. Scavenge its high-water back to the OS so a client-triggered + // reanalysis doesn't ratchet the idle footprint up and leave it there. + freeOSMemoryAfterBurst(s.logger, "mcp_analysis") +} + +// freeOSMemoryAfterBurst returns a completed whole-graph burst's heap +// high-water to the OS. debug.FreeOSMemory forces a GC + scavenge; +// GORTEX_DAEMON_MEMRELEASE=0 (or "false") disables it. The env check is +// duplicated here (rather than shared) because the canonical release helper +// lives in the cmd layer, which this package must not import. +func freeOSMemoryAfterBurst(logger *zap.Logger, reason string) { + if v := os.Getenv("GORTEX_DAEMON_MEMRELEASE"); v == "0" || strings.EqualFold(v, "false") { + return + } + start := time.Now() + debug.FreeOSMemory() + if logger != nil { + logger.Debug("mcp: released heap to OS", + zap.String("reason", reason), + zap.Duration("elapsed", time.Since(start))) + } } // resetConfirmedRefs clears the lazy-enrichment ledger (see the diff --git a/internal/platform/mem_darwin.go b/internal/platform/mem_darwin.go new file mode 100644 index 00000000..c6837e98 --- /dev/null +++ b/internal/platform/mem_darwin.go @@ -0,0 +1,17 @@ +//go:build darwin + +package platform + +import "golang.org/x/sys/unix" + +// HostPhysicalMemoryBytes returns total physical RAM in bytes via the +// hw.memsize sysctl. Returns 0 when the sysctl is unavailable, so a caller +// that derives a budget from it can fall back cleanly to "host RAM unknown" +// rather than acting on a bogus figure. +func HostPhysicalMemoryBytes() uint64 { + n, err := unix.SysctlUint64("hw.memsize") + if err != nil { + return 0 + } + return n +} diff --git a/internal/platform/mem_linux.go b/internal/platform/mem_linux.go new file mode 100644 index 00000000..393f17a0 --- /dev/null +++ b/internal/platform/mem_linux.go @@ -0,0 +1,22 @@ +//go:build linux + +package platform + +import "golang.org/x/sys/unix" + +// HostPhysicalMemoryBytes returns total physical RAM in bytes via +// sysinfo(2). Totalram is reported in units of Unit bytes; both fields are +// widened to uint64 so the multiply is correct on 32-bit arches too. +// Returns 0 when the syscall fails, so a caller that derives a budget from +// it can fall back cleanly to "host RAM unknown". +func HostPhysicalMemoryBytes() uint64 { + var si unix.Sysinfo_t + if err := unix.Sysinfo(&si); err != nil { + return 0 + } + unit := uint64(si.Unit) + if unit == 0 { + unit = 1 + } + return uint64(si.Totalram) * unit +} diff --git a/internal/platform/mem_other.go b/internal/platform/mem_other.go new file mode 100644 index 00000000..024e1999 --- /dev/null +++ b/internal/platform/mem_other.go @@ -0,0 +1,9 @@ +//go:build !linux && !darwin + +package platform + +// HostPhysicalMemoryBytes has no portable reader on this platform, so it +// returns 0. A caller then treats host RAM as unknown and skips any policy +// that needs it (e.g. a RAM-derived default memory limit) rather than +// guessing. Linux and darwin carry real implementations. +func HostPhysicalMemoryBytes() uint64 { return 0 } diff --git a/internal/platform/mem_test.go b/internal/platform/mem_test.go new file mode 100644 index 00000000..3609ae21 --- /dev/null +++ b/internal/platform/mem_test.go @@ -0,0 +1,26 @@ +package platform + +import ( + "runtime" + "testing" +) + +func TestHostPhysicalMemoryBytes(t *testing.T) { + got := HostPhysicalMemoryBytes() + + // Reject an implausibly large figure (a malformed reader masquerading + // as an exabyte of RAM) on every platform. + const oneEiB = uint64(1) << 60 + if got > oneEiB { + t.Fatalf("implausible host RAM: %d bytes", got) + } + + // linux and darwin carry real readers, so a live host must report a + // non-zero figure; other platforms deliberately return 0 (unknown). + switch runtime.GOOS { + case "linux", "darwin": + if got == 0 { + t.Fatalf("expected non-zero host RAM on %s", runtime.GOOS) + } + } +} diff --git a/internal/resolver/external_calls.go b/internal/resolver/external_calls.go index adadce3f..d58a1280 100644 --- a/internal/resolver/external_calls.go +++ b/internal/resolver/external_calls.go @@ -90,6 +90,22 @@ func SynthesizeExternalCallsForFiles(g graph.Store, enabled bool, files []string return synthesizeExternalCalls(g, func() []*graph.Edge { return externalCallCandidateEdgesForFiles(g, files) }) } +// SynthesizeExternalCallsForRepos is the repo-scoped counterpart used by the +// end-of-batch global passes when only some repos re-indexed: it materialises +// external-call nodes for the out-edges of the changed repos' symbols only, so +// the janitor pays O(changed-repo edges) instead of a whole-graph recompute. An +// external terminal always originates in the repo that made the call, so an +// unchanged repo's synthesised edges (already on disk, never dropped) need no +// re-work. The shared per-package nodes are deterministic, so a call into an +// already-materialised package dedups onto the existing node. A no-op when +// disabled or when no repo is in scope. +func SynthesizeExternalCallsForRepos(g graph.Store, enabled bool, prefixes map[string]bool) int { + if g == nil || !enabled || len(prefixes) == 0 { + return 0 + } + return synthesizeExternalCalls(g, func() []*graph.Edge { return externalCallCandidateEdgesForRepos(g, prefixes) }) +} + // synthesizeExternalCalls is the shared materialisation core. collect runs // under the resolve lock and returns the candidate call / reference edges // (external-package terminals plus any already-synthesised external-call:: @@ -244,6 +260,32 @@ func externalCallCandidateEdgesForFiles(g graph.Store, files []string) []*graph. return out } +// externalCallCandidateEdgesForRepos returns the external-terminal call / +// reference out-edges originating in the given changed repos — the O(changed +// repo) input for the end-of-batch scoped synthesis. GetRepoEdges is one +// backend query per repo (the out-edges of every symbol the repo defines), so +// this never materialises the whole graph's call edges. +func externalCallCandidateEdgesForRepos(g graph.Store, prefixes map[string]bool) []*graph.Edge { + var out []*graph.Edge + for prefix := range prefixes { + if prefix == "" { + continue + } + for _, e := range g.GetRepoEdges(prefix) { + if e == nil { + continue + } + if e.Kind != graph.EdgeCalls && e.Kind != graph.EdgeReferences { + continue + } + if isExternalCandidateTarget(e.To) { + out = append(out, e) + } + } + } + return out +} + // isExternalCandidateTarget reports whether a target string is one that // synthesizeExternalCalls considers: an external-package terminal or an // already-materialised external-call:: node (kept so the pass's return diff --git a/internal/resolver/fn_value_gate.go b/internal/resolver/fn_value_gate.go index 5392697b..39fabcd5 100644 --- a/internal/resolver/fn_value_gate.go +++ b/internal/resolver/fn_value_gate.go @@ -43,7 +43,24 @@ const ( // full-recompute, idempotent synthesizer: graph.AddEdge dedupes and // graph.EvictFile drops the edges on reindex. Returns the number of edges // landed. -func ResolveFnValueCallbacks(g graph.Store) int { +func ResolveFnValueCallbacks(g graph.Store) int { return resolveFnValueCallbacks(g, nil) } + +// ResolveFnValueCallbacksScoped is the incremental counterpart of +// ResolveFnValueCallbacks: it gates only the callback candidates that originate +// in the given changed repos, leaving an unchanged repo's already-bound +// registrations on disk (they were never dropped). A nil scope gates the whole +// graph, so ResolveFnValueCallbacks and the whole-index path stay identical. +// +// Only the CANDIDATE scan is scoped. A candidate placeholder lives in (is +// emitted from) the repo that declared the registration, so a changed repo owns +// exactly the candidates whose binding its reindex dropped. RESOLUTION stays +// whole-graph — the resolve helpers below scan the entire graph by name — so a +// changed-repo callback still binds to a handler that lives in an unchanged repo. +func ResolveFnValueCallbacksScoped(g graph.Store, scope map[string]bool) int { + return resolveFnValueCallbacks(g, scope) +} + +func resolveFnValueCallbacks(g graph.Store, scope map[string]bool) int { if g == nil { return 0 } @@ -63,29 +80,24 @@ func ResolveFnValueCallbacks(g graph.Store) int { fileNodes[filePath] = ns return ns } - // The gate needs only the placeholders parked in the fn-value namespace, - // not every reference edge. When the backend can range-scan that namespace - // (FnValuePlaceholderScanner) use it: the generic EdgesByKind(references) - // path materialises the whole placeholders-plus-real-references set on every - // whole-graph synthesizer pass — several times the size of the placeholder - // slice on a large multi-repo graph. Both iterators are iter.Seq[*Edge], so - // the loop body is identical; the Meta["via"] == callback_candidate filter - // below STAYS on both paths — a non-candidate edge can be parked in the - // namespace (e.g. an already-bound registration) and must never be gated. - edges := g.EdgesByKind(graph.EdgeReferences) - if fp, ok := g.(graph.FnValuePlaceholderScanner); ok { - edges = fp.FnValuePlaceholderEdges() - } - for e := range edges { + // nameMemo caches g.FindNodesByName(name) for the life of the pass. The + // resolve helpers hit it repeatedly for the same registration name (every + // router.Get("/x", handler) that names the same handler, every recurring + // Class::method string), and each hit was an unmemoized FindNodesByName — + // on a large graph the single largest cost of the gate. No node is added or + // removed until the AddEdge tail below, so a name's node set is stable + // across the pass and the memo returns identical results. + nameMemo := map[string][]*graph.Node{} + process := func(e *graph.Edge) { if e == nil || e.Meta == nil { - continue + return } if via, _ := e.Meta["via"].(string); via != fnValueCandidateVia { - continue + return } name, _ := e.Meta[metaFnValueName].(string) if name == "" || isFnValueNonTarget(name) { - continue + return } // Resolution scope depends on the captured form. A special form's // receiver hint (`` / a concrete type) binds the member against @@ -104,29 +116,29 @@ func ResolveFnValueCallbacks(g graph.Store) int { // repo-wide unique-or-drop rule (a `Class::method` string scopes to // the type). if recvHint != "" { - target = resolveMemberByType(g, recvHint, name) + target = resolveMemberByTypeMemo(g, recvHint, name, nameMemo) } if target == "" { - target = resolveUniqueFnValue(g, name) + target = resolveUniqueFnValueMemo(g, name, nameMemo) } conf = 0.5 case recvHint == "": - if target = resolveFnValueSelfMember(g, e.From, name); target != "" { + if target = resolveFnValueSelfMemberMemo(g, e.From, name, nameMemo); target != "" { conf, origin = 0.85, graph.OriginASTResolved } else { target = resolveFnValueName(getFileNodes(e.FilePath), name) } case recvHint != "": - if target = resolveMemberByType(g, recvHint, name); target != "" { + if target = resolveMemberByTypeMemo(g, recvHint, name, nameMemo); target != "" { conf, origin = 0.85, graph.OriginASTResolved } else if ungated { - target = resolveFnValueCrossModule(g, name) + target = resolveFnValueCrossModuleMemo(g, name, nameMemo) conf = 0.45 } default: target = resolveFnValueName(getFileNodes(e.FilePath), name) if target == "" && ungated { - target = resolveFnValueCrossModule(g, name) + target = resolveFnValueCrossModuleMemo(g, name, nameMemo) conf = 0.45 } } @@ -134,7 +146,7 @@ func ResolveFnValueCallbacks(g graph.Store) int { // Unbound (a local / param / undefined name) or a self-reference // (a function's own declaration token): reject rather than // fabricate an edge. - continue + return } meta := map[string]any{ "via": fnValueRegistrationVia, @@ -157,6 +169,40 @@ func ResolveFnValueCallbacks(g graph.Store) int { Meta: meta, }) } + + if scope == nil { + // The gate needs only the placeholders parked in the fn-value namespace, + // not every reference edge. When the backend can range-scan that namespace + // (FnValuePlaceholderScanner) use it: the generic EdgesByKind(references) + // path materialises the whole placeholders-plus-real-references set on every + // whole-graph synthesizer pass — several times the size of the placeholder + // slice on a large multi-repo graph. Both iterators are iter.Seq[*Edge], so + // the loop body is identical; the Meta["via"] == callback_candidate filter + // in process STAYS on both paths — a non-candidate edge can be parked in the + // namespace (e.g. an already-bound registration) and must never be gated. + edges := g.EdgesByKind(graph.EdgeReferences) + if fp, ok := g.(graph.FnValuePlaceholderScanner); ok { + edges = fp.FnValuePlaceholderEdges() + } + for e := range edges { + process(e) + } + } else { + // Scoped: walk only the changed repos' out-edges (GetRepoEdges is one + // backend query per repo). The via filter in process still applies, so a + // non-candidate reference edge in the changed repo is ignored. + for prefix := range scope { + if prefix == "" { + continue + } + for _, e := range g.GetRepoEdges(prefix) { + if e == nil || e.Kind != graph.EdgeReferences { + continue + } + process(e) + } + } + } for _, e := range landed { g.AddEdge(e) } @@ -190,24 +236,49 @@ func resolveFnValueName(fileNodes []*graph.Node, name string) string { // the repo, or "" when none or more than one exists (unique-or-drop). The // shared repo-wide resolution rule for qualified-path and gate-skipping // (curated-HOF string) function values. Prototype declarations of the name -// never make it ambiguous — see uniqueFnValueMatch. +// never make it ambiguous — see uniqueFnValueMatchMemo. func resolveUniqueFnValue(g graph.Store, name string) string { - return uniqueFnValueMatch(g, name, nil) + return resolveUniqueFnValueMemo(g, name, nil) } -// resolveFnValueCrossModule binds a function value to a uniquely-named +// resolveUniqueFnValueMemo is resolveUniqueFnValue with a shared per-pass +// FindNodesByName memo (nil disables memoization). +func resolveUniqueFnValueMemo(g graph.Store, name string, memo map[string][]*graph.Node) string { + return uniqueFnValueMatchMemo(g, name, nil, memo) +} + +// resolveFnValueCrossModuleMemo binds a function value to a uniquely-named // function/method anywhere in the repo, skipping any candidate with file-local // linkage (a C/C++ `static` function, stamped scope_static): such a definition // is invisible outside its translation unit, so a cross-module reference can // never target it, and a same-named static in an unrelated file must not make // the name look ambiguous. The same-file path is preferred by the caller; this -// is the cross-module fallback. -func resolveFnValueCrossModule(g graph.Store, name string) string { - return uniqueFnValueMatch(g, name, isFileLocalLinkage) +// is the cross-module fallback. A shared per-pass FindNodesByName memo collapses +// repeated lookups of the same name (nil disables memoization). +func resolveFnValueCrossModuleMemo(g graph.Store, name string, memo map[string][]*graph.Node) string { + return uniqueFnValueMatchMemo(g, name, isFileLocalLinkage, memo) +} + +// findNodesByNameMemo wraps g.FindNodesByName with an optional per-pass cache. +// The gate calls it for the same registration names many times; caching the +// result collapses those to one backend lookup per distinct name. Safe only +// within a pass that does not add or remove nodes between lookups. A nil memo +// forwards straight through, so non-pass callers see identical behaviour. +func findNodesByNameMemo(g graph.Store, name string, memo map[string][]*graph.Node) []*graph.Node { + if memo == nil { + return g.FindNodesByName(name) + } + if ns, ok := memo[name]; ok { + return ns + } + ns := g.FindNodesByName(name) + memo[name] = ns + return ns } -// uniqueFnValueMatch is the shared unique-or-drop scan over every -// function/method named name, with an optional per-node exclusion. +// uniqueFnValueMatchMemo is the shared unique-or-drop scan over every +// function/method named name, with an optional per-node exclusion and a shared +// per-pass FindNodesByName memo (nil disables memoization). // // A C-family forward declaration (`void strlenCommand(client *c);` in a // header, stamped Meta["prototype"]) names the SAME extern symbol as its @@ -220,9 +291,9 @@ func resolveFnValueCrossModule(g graph.Store, name string) string { // prototypes are consulted only when no definition matches at all (the // definition's translation unit isn't indexed), and then under the same // unique-or-drop rule. -func uniqueFnValueMatch(g graph.Store, name string, exclude func(*graph.Node) bool) string { +func uniqueFnValueMatchMemo(g graph.Store, name string, exclude func(*graph.Node) bool, memo map[string][]*graph.Node) string { def, proto := "", "" - for _, n := range g.FindNodesByName(name) { + for _, n := range findNodesByNameMemo(g, name, memo) { if n == nil { continue } @@ -284,11 +355,17 @@ func isFileLocalLinkage(n *graph.Node) bool { // (matched via Meta["receiver"]), or "" when none or more than one matches. // Shared scope rule for `Foo::bar`-style references and self-member resolution. func resolveMemberByType(g graph.Store, typeName, member string) string { + return resolveMemberByTypeMemo(g, typeName, member, nil) +} + +// resolveMemberByTypeMemo is resolveMemberByType with a shared per-pass +// FindNodesByName memo (nil disables memoization). +func resolveMemberByTypeMemo(g graph.Store, typeName, member string, memo map[string][]*graph.Node) string { if typeName == "" || member == "" { return "" } match := "" - for _, n := range g.FindNodesByName(member) { + for _, n := range findNodesByNameMemo(g, member, memo) { if n == nil || n.Kind != graph.KindMethod { continue } @@ -303,10 +380,11 @@ func resolveMemberByType(g graph.Store, typeName, member string) string { return match } -// resolveFnValueSelfMember binds a `this.m` / `self.m` member reference against -// the methods of the registration site's enclosing type, so it can never bind -// a coincidentally-named top-level function. -func resolveFnValueSelfMember(g graph.Store, fromID, member string) string { +// resolveFnValueSelfMemberMemo binds a `this.m` / `self.m` member reference +// against the methods of the registration site's enclosing type, so it can +// never bind a coincidentally-named top-level function. A shared per-pass +// FindNodesByName memo collapses repeated lookups (nil disables memoization). +func resolveFnValueSelfMemberMemo(g graph.Store, fromID, member string, memo map[string][]*graph.Node) string { from := g.GetNode(fromID) if from == nil || from.Meta == nil { return "" @@ -315,7 +393,7 @@ func resolveFnValueSelfMember(g graph.Store, fromID, member string) string { if recv == "" { return "" } - return resolveMemberByType(g, recv, member) + return resolveMemberByTypeMemo(g, recv, member, memo) } // isFnValueNonTarget reports whether name is a literal/keyword/builtin that diff --git a/internal/resolver/framework_synth.go b/internal/resolver/framework_synth.go index ee1771de..02f674ac 100644 --- a/internal/resolver/framework_synth.go +++ b/internal/resolver/framework_synth.go @@ -175,14 +175,31 @@ func UnstampSynthesized(e *graph.Edge) { // synthFunc adapts a plain pass function into a FrameworkSynthesizer so // the existing passes (ResolveGRPCStubCalls, …) register without a // wrapper type each. +// +// scopedFn is optional: when set, the end-of-batch driver calls it with the +// changed-repo prefix set so the synthesizer can restrict its CANDIDATE scan to +// those repos (resolution stays whole-graph). A synthesizer without a scopedFn +// always runs whole-graph — correct, just not narrowed — so scoping any one +// pass is opt-in and additive. type synthFunc struct { - name string - fn func(graph.Store) int + name string + fn func(graph.Store) int + scopedFn func(graph.Store, map[string]bool) int } func (s synthFunc) Name() string { return s.name } func (s synthFunc) Synthesize(g graph.Store) int { return s.fn(g) } +// synthesizeScoped runs the scoped variant when one is registered and a scope +// is armed; otherwise it falls back to the whole-graph pass. A nil scope always +// means whole-graph, so the fresh-index path is byte-identical to before. +func (s synthFunc) synthesizeScoped(g graph.Store, scope map[string]bool) int { + if scope != nil && s.scopedFn != nil { + return s.scopedFn(g, scope) + } + return s.fn(g) +} + // defaultFrameworkSynthesizers returns the registered framework // synthesizers in run order. Order is load-bearing: every synthesizer // here runs after InferImplements/InferOverrides (some depend on the @@ -308,12 +325,12 @@ func defaultFrameworkSynthesizers() []FrameworkSynthesizer { // value-position function identifier to its same-file definition and // drops unbound candidates. The per-language capture feeds it via // placeholder edges; the pass is inert until those land. - synthFunc{name: SynthFnValue, fn: ResolveFnValueCallbacks}, + synthFunc{name: SynthFnValue, fn: ResolveFnValueCallbacks, scopedFn: ResolveFnValueCallbacksScoped}, // Pascal unit ↔ form (.pas/.dfm) pairing by same-dir basename. synthFunc{name: SynthPascalFormName, fn: ResolvePascalForms}, // Same-file distinctive value references → EdgeReads to the constant, // so a config constant's blast radius reaches every reader. - synthFunc{name: SynthValueRefName, fn: ResolveValueRefs}, + synthFunc{name: SynthValueRefName, fn: ResolveValueRefs, scopedFn: ResolveValueRefsScoped}, } } @@ -349,17 +366,41 @@ type FrameworkSynthReport struct { DemoteMillis int64 `json:"demote_ms,omitempty"` } +// scopedSynthesizer is the optional capability a FrameworkSynthesizer exposes +// when it can restrict its candidate scan to a changed-repo prefix set. The +// driver consults it only when a scope is armed; a synthesizer that does not +// implement it runs whole-graph, which is always correct. +type scopedSynthesizer interface { + synthesizeScoped(g graph.Store, scope map[string]bool) int +} + // RunFrameworkSynthesizers runs every registered framework synthesizer // over g, in registration order, and returns the per-synthesizer and // total landed-edge counts. A nil graph is a no-op. func RunFrameworkSynthesizers(g graph.Store) FrameworkSynthReport { + return RunFrameworkSynthesizersScoped(g, nil) +} + +// RunFrameworkSynthesizersScoped is RunFrameworkSynthesizers with an armed +// changed-repo scope: each synthesizer that implements scopedSynthesizer +// narrows its candidate scan to those repos, the rest run whole-graph. A nil +// scope runs every pass whole-graph, so the fresh-index / single-repo path is +// byte-identical to the pre-scoping behaviour. The claiming-resolver, family- +// gate and receiver-gate tail passes always run whole-graph — they reconcile +// the settled cross-repo call graph, not a per-repo candidate set. +func RunFrameworkSynthesizersScoped(g graph.Store, scope map[string]bool) FrameworkSynthReport { rep := FrameworkSynthReport{} if g == nil { return rep } for _, s := range defaultFrameworkSynthesizers() { start := time.Now() - n := s.Synthesize(g) + var n int + if ss, ok := s.(scopedSynthesizer); ok { + n = ss.synthesizeScoped(g, scope) + } else { + n = s.Synthesize(g) + } rep.Per = append(rep.Per, SynthCount{Name: s.Name(), Edges: n, Millis: time.Since(start).Milliseconds()}) rep.Total += n } diff --git a/internal/resolver/scoped_global_passes_test.go b/internal/resolver/scoped_global_passes_test.go new file mode 100644 index 00000000..974170e9 --- /dev/null +++ b/internal/resolver/scoped_global_passes_test.go @@ -0,0 +1,281 @@ +package resolver + +import ( + "iter" + "testing" + + "github.com/zzet/gortex/internal/graph" +) + +// countingStore wraps a graph.Store and counts the read paths the scoped global +// passes take, so a test can prove a scoped run drives off the per-repo readers +// (GetRepoEdges / GetRepoNodes) instead of the whole-graph scans and materialises +// fewer nodes. It deliberately does NOT re-expose the optional capabilities +// (FnValuePlaceholderScanner, NodesByKindsScanner, …) of the wrapped store, so +// both the scoped and unscoped runs under test take the same generic fallback +// paths and the comparison isolates the scope effect. +type countingStore struct { + graph.Store + edgesByKind int + repoEdges map[string]int + nodesReturned int // total *Node materialised via node-returning reads +} + +func newCountingStore(s graph.Store) *countingStore { + return &countingStore{Store: s, repoEdges: map[string]int{}} +} + +func (c *countingStore) EdgesByKind(k graph.EdgeKind) iter.Seq[*graph.Edge] { + c.edgesByKind++ + return c.Store.EdgesByKind(k) +} + +func (c *countingStore) GetRepoEdges(prefix string) []*graph.Edge { + c.repoEdges[prefix]++ + return c.Store.GetRepoEdges(prefix) +} + +func (c *countingStore) GetFileNodes(path string) []*graph.Node { + ns := c.Store.GetFileNodes(path) + c.nodesReturned += len(ns) + return ns +} + +func (c *countingStore) FindNodesByName(name string) []*graph.Node { + ns := c.Store.FindNodesByName(name) + c.nodesReturned += len(ns) + return ns +} + +func (c *countingStore) NodesByKind(k graph.NodeKind) iter.Seq[*graph.Node] { + inner := c.Store.NodesByKind(k) + return func(yield func(*graph.Node) bool) { + for n := range inner { + c.nodesReturned++ + if !yield(n) { + return + } + } + } +} + +func overrideEdgeSet(g graph.Store) map[string]bool { + out := map[string]bool{} + for _, e := range g.AllEdges() { + if e.Kind == graph.EdgeOverrides { + out[e.From+"->"+e.To] = true + } + } + return out +} + +// crossRepoOverrideFixture builds a child type in repo A whose supertype lives +// in the (unchanged) repo B, so the override edge Derived.Do -> Base.Do spans a +// repo boundary. InferOverrides has no same-repo gate, so this pair is real. +func crossRepoOverrideFixture() *graph.Graph { + g := graph.New() + // Repo B (unchanged): the parent. + g.AddNode(&graph.Node{ID: "B::b.go::Base", Kind: graph.KindType, Name: "Base", RepoPrefix: "B", FilePath: "b.go"}) + g.AddNode(&graph.Node{ID: "B::b.go::Base.Do", Kind: graph.KindMethod, Name: "Do", RepoPrefix: "B", FilePath: "b.go"}) + g.AddEdge(&graph.Edge{From: "B::b.go::Base.Do", To: "B::b.go::Base", Kind: graph.EdgeMemberOf}) + // Repo A (changed): the child extending the cross-repo parent. + g.AddNode(&graph.Node{ID: "A::a.go::Derived", Kind: graph.KindType, Name: "Derived", RepoPrefix: "A", FilePath: "a.go"}) + g.AddNode(&graph.Node{ID: "A::a.go::Derived.Do", Kind: graph.KindMethod, Name: "Do", RepoPrefix: "A", FilePath: "a.go"}) + g.AddEdge(&graph.Edge{From: "A::a.go::Derived.Do", To: "A::a.go::Derived", Kind: graph.EdgeMemberOf}) + g.AddEdge(&graph.Edge{From: "A::a.go::Derived", To: "B::b.go::Base", Kind: graph.EdgeExtends, Origin: graph.OriginASTResolved}) + return g +} + +// TestInferOverridesScoped_CrossRepoBoundary asserts a scope that names ONLY the +// changed repo's types still re-derives a cross-repo override whose parent lives +// in an unchanged repo — the filter keeps a parent-edge row when EITHER endpoint +// is in scope, so the changed child is enough. +func TestInferOverridesScoped_CrossRepoBoundary(t *testing.T) { + full := crossRepoOverrideFixture() + New(full).InferOverrides() + want := overrideEdgeSet(full) + if len(want) != 1 { + t.Fatalf("setup: expected 1 cross-repo override from full pass, got %d: %v", len(want), want) + } + + // Scope = repo A's type/interface IDs only (repo B did not change). The + // parent Base lives in B and is NOT in scope; the scoped pass must still + // re-derive Derived.Do -> Base.Do because the child Derived is in scope. + scoped := crossRepoOverrideFixture() + New(scoped).InferOverridesScoped(map[string]bool{"A::a.go::Derived": true}) + got := overrideEdgeSet(scoped) + if len(got) != len(want) { + t.Fatalf("scoped cross-repo override = %v, want %v", got, want) + } + for k := range want { + if !got[k] { + t.Errorf("scoped run dropped cross-repo override %q", k) + } + } +} + +func fnValueCandidate(from, filePath, name string) *graph.Edge { + return &graph.Edge{ + From: from, + To: "unresolved::fnvalue::" + name, + Kind: graph.EdgeReferences, + FilePath: filePath, + Meta: map[string]any{ + "via": fnValueCandidateVia, + metaFnValueName: name, + "fn_value_ungated": true, + }, + } +} + +func callbackEdgeSet(g graph.Store) map[string]bool { + out := map[string]bool{} + for _, e := range g.AllEdges() { + if e.Kind != graph.EdgeReferences || e.Meta == nil { + continue + } + if via, _ := e.Meta["via"].(string); via == fnValueRegistrationVia { + out[e.From+"->"+e.To] = true + } + } + return out +} + +// crossRepoFnValueFixture builds a callback registration in repo A whose handler +// is a uniquely-named function in the (unchanged) repo B, plus a second +// registration wholly inside repo B. The scoped run over {A} must still bind +// A's callback to the B handler (resolution is whole-graph) while never touching +// B's own candidate. +func crossRepoFnValueFixture() *graph.Graph { + g := graph.New() + // Repo B: two handler functions plus a B-local callback registration. + g.AddNode(&graph.Node{ID: "B::b.go::HandlerB", Kind: graph.KindFunction, Name: "HandlerB", RepoPrefix: "B", FilePath: "b.go"}) + g.AddNode(&graph.Node{ID: "B::b.go::HandlerB2", Kind: graph.KindFunction, Name: "HandlerB2", RepoPrefix: "B", FilePath: "b.go"}) + g.AddNode(&graph.Node{ID: "B::b.go::RegisterB", Kind: graph.KindFunction, Name: "RegisterB", RepoPrefix: "B", FilePath: "b.go"}) + g.AddEdge(fnValueCandidate("B::b.go::RegisterB", "b.go", "HandlerB2")) + // Repo A: a registration referencing the cross-repo handler by name. + g.AddNode(&graph.Node{ID: "A::a.go::RegisterA", Kind: graph.KindFunction, Name: "RegisterA", RepoPrefix: "A", FilePath: "a.go"}) + g.AddEdge(fnValueCandidate("A::a.go::RegisterA", "a.go", "HandlerB")) + return g +} + +// TestResolveFnValueCallbacksScoped_CrossRepoHandler asserts the scoped fn-value +// gate binds a changed repo's callback to a handler in an UNCHANGED repo (the +// resolution side stays whole-graph), drives its candidate scan off GetRepoEdges +// rather than the whole-graph EdgeReferences scan, and materialises fewer nodes +// than the unscoped run (it never resolves the unchanged repo's own candidate). +func TestResolveFnValueCallbacksScoped_CrossRepoHandler(t *testing.T) { + // Unscoped: both candidates bind. + full := newCountingStore(crossRepoFnValueFixture()) + if n := ResolveFnValueCallbacks(full); n != 2 { + t.Fatalf("unscoped fn-value: expected 2 bound callbacks, got %d", n) + } + fullEdges := callbackEdgeSet(full) + if !fullEdges["A::a.go::RegisterA->B::b.go::HandlerB"] { + t.Fatalf("unscoped did not bind the cross-repo callback: %v", fullEdges) + } + if full.edgesByKind == 0 { + t.Errorf("unscoped fn-value should scan EdgesByKind(references) whole-graph") + } + + // Scoped over {A}: only A's candidate is gated, but it still binds to the B + // handler via the whole-graph name resolution. + scoped := newCountingStore(crossRepoFnValueFixture()) + if n := ResolveFnValueCallbacksScoped(scoped, map[string]bool{"A": true}); n != 1 { + t.Fatalf("scoped fn-value over {A}: expected 1 bound callback, got %d", n) + } + scopedEdges := callbackEdgeSet(scoped) + if !scopedEdges["A::a.go::RegisterA->B::b.go::HandlerB"] { + t.Errorf("scoped run dropped the cross-repo callback binding: %v", scopedEdges) + } + if scopedEdges["B::b.go::RegisterB->B::b.go::HandlerB2"] { + t.Errorf("scoped run must NOT re-bind the unchanged repo's own candidate: %v", scopedEdges) + } + + // Candidate scan path: scoped drives off GetRepoEdges(A), never the + // whole-graph EdgeReferences scan. + if scoped.edgesByKind != 0 { + t.Errorf("scoped fn-value must not scan EdgesByKind whole-graph, got %d calls", scoped.edgesByKind) + } + if scoped.repoEdges["A"] == 0 { + t.Errorf("scoped fn-value must scan GetRepoEdges(\"A\")") + } + // Fewer node reads: scoped skips resolving B's candidate entirely. + if scoped.nodesReturned >= full.nodesReturned { + t.Errorf("scoped run should materialise fewer nodes than unscoped: scoped=%d full=%d", + scoped.nodesReturned, full.nodesReturned) + } +} + +func mkValueRefCandidate(from, filePath, name string) *graph.Edge { + return &graph.Edge{ + From: from, + To: "unresolved::value::" + name, + Kind: graph.EdgeReads, + FilePath: filePath, + Meta: map[string]any{"via": valueRefCandidateVia, "name": name}, + } +} + +// boundValueRefs maps reader -> constant for every value-ref read the pass bound +// (via == value_ref); a still-unbound candidate keeps the value_ref_candidate +// marker and is absent from the map. +func boundValueRefs(g graph.Store) map[string]string { + out := map[string]string{} + for _, e := range g.AllEdges() { + if e.Kind != graph.EdgeReads || e.Meta == nil { + continue + } + if via, _ := e.Meta["via"].(string); via == valueRefVia { + out[e.From] = e.To + } + } + return out +} + +func crossRepoValueRefFixture() *graph.Graph { + g := graph.New() + // Repo A (changed): distinctive constant + a same-file reader candidate. + g.AddNode(&graph.Node{ID: "A::a.go::MAX_SIZE", Kind: graph.KindConstant, Name: "MAX_SIZE", RepoPrefix: "A", FilePath: "a.go", StartLine: 1}) + g.AddNode(&graph.Node{ID: "A::a.go::useA", Kind: graph.KindFunction, Name: "useA", RepoPrefix: "A", FilePath: "a.go", StartLine: 5}) + g.AddEdge(mkValueRefCandidate("A::a.go::useA", "a.go", "MAX_SIZE")) + // Repo B (unchanged): its own distinctive constant + same-file reader. + g.AddNode(&graph.Node{ID: "B::b.go::MIN_SIZE", Kind: graph.KindConstant, Name: "MIN_SIZE", RepoPrefix: "B", FilePath: "b.go", StartLine: 1}) + g.AddNode(&graph.Node{ID: "B::b.go::useB", Kind: graph.KindFunction, Name: "useB", RepoPrefix: "B", FilePath: "b.go", StartLine: 5}) + g.AddEdge(mkValueRefCandidate("B::b.go::useB", "b.go", "MIN_SIZE")) + return g +} + +// TestResolveValueRefsScoped_CrossRepo asserts the scoped value-ref pass binds +// only the changed repo's candidate (leaving the unchanged repo's on-disk +// binding untouched), and drives its candidate scan off GetRepoEdges rather than +// the whole-graph EdgeReads scan. The unscoped pass binds both, proving the +// scoped pass is a strict narrowing of the same resolution. +func TestResolveValueRefsScoped_CrossRepo(t *testing.T) { + full := crossRepoValueRefFixture() + if n := ResolveValueRefs(full); n != 2 { + t.Fatalf("unscoped value-ref: expected 2 bound reads, got %d", n) + } + fb := boundValueRefs(full) + if fb["A::a.go::useA"] != "A::a.go::MAX_SIZE" || fb["B::b.go::useB"] != "B::b.go::MIN_SIZE" { + t.Fatalf("unscoped value-ref bindings wrong: %v", fb) + } + + scoped := newCountingStore(crossRepoValueRefFixture()) + if n := ResolveValueRefsScoped(scoped, map[string]bool{"A": true}); n != 1 { + t.Fatalf("scoped value-ref over {A}: expected 1 bound read, got %d", n) + } + sb := boundValueRefs(scoped) + if sb["A::a.go::useA"] != "A::a.go::MAX_SIZE" { + t.Errorf("scoped run dropped repo A's value-ref binding: %v", sb) + } + if _, ok := sb["B::b.go::useB"]; ok { + t.Errorf("scoped run must not bind the unchanged repo B's value-ref: %v", sb) + } + if scoped.edgesByKind != 0 { + t.Errorf("scoped value-ref must not scan EdgesByKind whole-graph, got %d calls", scoped.edgesByKind) + } + if scoped.repoEdges["A"] == 0 { + t.Errorf("scoped value-ref must scan GetRepoEdges(\"A\")") + } +} diff --git a/internal/resolver/value_refs.go b/internal/resolver/value_refs.go index 861fa9c4..3c6e91df 100644 --- a/internal/resolver/value_refs.go +++ b/internal/resolver/value_refs.go @@ -35,38 +35,71 @@ const ( // Unresolved candidates are // left as inert placeholders. Idempotent: re-targeting to the same constant is // a no-op and graph.EvictFile drops the edges on reindex. -func ResolveValueRefs(g graph.Store) int { +func ResolveValueRefs(g graph.Store) int { return resolveValueRefs(g, nil) } + +// ResolveValueRefsScoped is the incremental counterpart of ResolveValueRefs: it +// resolves only the value-ref candidates that originate in the given changed +// repos, leaving an unchanged repo's already-bound reads on disk (they were +// never dropped). A nil scope resolves the whole graph, so ResolveValueRefs and +// the whole-index path stay byte-identical. Restricting the candidate set is +// binding-preserving: every candidate binds to a constant declared in its OWN +// file, so which other repos are in scope can never change a resolution. +func ResolveValueRefsScoped(g graph.Store, scope map[string]bool) int { + return resolveValueRefs(g, scope) +} + +func resolveValueRefs(g graph.Store, scope map[string]bool) int { if g == nil { return 0 } + // Two-pass narrowing. A value-ref binds a captured read to a file-scope + // constant declared in the SAME file, so only the files that actually carry + // a candidate read need their declarators indexed. Gather the candidate + // edges first (Pass A), then build the constant/local maps for their files + // alone (Pass B) — instead of a whole-graph scan over every + // constant/variable/param/field/local node, which is the largest node + // population in the graph and was materialised twice (a flat slice plus the + // nested maps) on every whole-graph run. + candidates := valueRefCandidateEdges(g, scope) + if len(candidates) == 0 { + return 0 + } + candidateFiles := make(map[string]struct{}, len(candidates)) + for _, e := range candidates { + if e.FilePath != "" { + candidateFiles[e.FilePath] = struct{}{} + } + } // constsByFile records every file-scope constant/variable declarator of a // distinctive name (a name may have several — a try/except import, a // `#[cfg]` const, an `#ifdef #define`); localsByFile records the // param/field/local declarators that may shadow a read in their own scope. constsByFile := map[string]map[string][]*graph.Node{} localsByFile := map[string]map[string][]*graph.Node{} - for _, n := range nodesByKindsOrAll(g, graph.KindConstant, graph.KindVariable, graph.KindParam, graph.KindField, graph.KindLocal) { - if n == nil || n.FilePath == "" { - continue - } - switch n.Kind { - case graph.KindConstant, graph.KindVariable: - if !isDistinctiveValueName(n.Name) { + for f := range candidateFiles { + for _, n := range g.GetFileNodes(f) { + if n == nil || n.FilePath == "" { continue } - m := constsByFile[n.FilePath] - if m == nil { - m = map[string][]*graph.Node{} - constsByFile[n.FilePath] = m - } - m[n.Name] = append(m[n.Name], n) - case graph.KindParam, graph.KindField, graph.KindLocal: - m := localsByFile[n.FilePath] - if m == nil { - m = map[string][]*graph.Node{} - localsByFile[n.FilePath] = m + switch n.Kind { + case graph.KindConstant, graph.KindVariable: + if !isDistinctiveValueName(n.Name) { + continue + } + m := constsByFile[n.FilePath] + if m == nil { + m = map[string][]*graph.Node{} + constsByFile[n.FilePath] = m + } + m[n.Name] = append(m[n.Name], n) + case graph.KindParam, graph.KindField, graph.KindLocal: + m := localsByFile[n.FilePath] + if m == nil { + m = map[string][]*graph.Node{} + localsByFile[n.FilePath] = m + } + m[n.Name] = append(m[n.Name], n) } - m[n.Name] = append(m[n.Name], n) } } if len(constsByFile) == 0 { @@ -80,13 +113,7 @@ func ResolveValueRefs(g graph.Store) int { resolved := 0 var reindex []graph.EdgeReindex - for e := range g.EdgesByKind(graph.EdgeReads) { - if e == nil || e.Meta == nil { - continue - } - if via, _ := e.Meta["via"].(string); via != valueRefCandidateVia { - continue - } + for _, e := range candidates { name, _ := e.Meta["name"].(string) consts := constsByFile[e.FilePath][name] if name == "" || len(consts) == 0 { @@ -138,6 +165,42 @@ func ResolveValueRefs(g graph.Store) int { return resolved } +// valueRefCandidateEdges returns the extractor-emitted placeholder read edges +// (Meta via == value_ref_candidate) the pass resolves. With a nil scope it +// scans every EdgeReads edge in the graph; with a scope it walks only the +// out-edges of the changed repos' nodes (GetRepoEdges is one backend query per +// repo), since a candidate read always originates in the repo that declared it. +func valueRefCandidateEdges(g graph.Store, scope map[string]bool) []*graph.Edge { + var out []*graph.Edge + keep := func(e *graph.Edge) { + if e == nil || e.Meta == nil { + return + } + if via, _ := e.Meta["via"].(string); via != valueRefCandidateVia { + return + } + out = append(out, e) + } + if scope == nil { + for e := range g.EdgesByKind(graph.EdgeReads) { + keep(e) + } + return out + } + for prefix := range scope { + if prefix == "" { + continue + } + for _, e := range g.GetRepoEdges(prefix) { + if e == nil || e.Kind != graph.EdgeReads { + continue + } + keep(e) + } + } + return out +} + // valueRefReaderShadowed reports whether any same-named declarator is scoped // inside the reading function — its node ID nests under the reader's ID via a // `.` / `#` / `:` scope separator (`f.go::Run.x`, `f.go::Run#x`). Such a