From e3bc3cbf5c1ee1903ff264815d2f8edfedde06fc Mon Sep 17 00:00:00 2001 From: Andrey Kumanyaev Date: Sat, 11 Jul 2026 20:29:06 +0200 Subject: [PATCH 1/7] perf(config): cache per-repo .gitignore parse and layered exclude merge MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit EffectiveExclude re-read and re-parsed the repo root .gitignore on every call — a fresh 64 KiB scanner buffer plus a string per line, on the indexer-walk / per-file-reconcile firehose. Live pprof of a long-running daemon attributed 47% of all allocations to loadRepoGitignore, which kept the GC cycling even at idle. Memoize both layers on the ConfigManager: - the .gitignore parse is cached per repo root and invalidated by an os.Stat mtime+size comparison, so a steady-state call does one stat and no file read - the fully layered exclude list is cached per repo prefix, versioned by pointer identity of the global/workspace configs (they are only ever swapped, never mutated in place) plus the .gitignore stat - the returned slice is shared and immutable by contract, clipped to len == cap so an appending caller reallocates instead of writing through the shared backing array Cache-hit path: 34147 -> 2182 ns/op, 68672 -> 432 B/op, 30 -> 3 allocs/op. --- internal/config/gitignore_cache.go | 162 ++++++++++++++ internal/config/gitignore_cache_test.go | 272 ++++++++++++++++++++++++ internal/config/manager.go | 43 +++- 3 files changed, 471 insertions(+), 6 deletions(-) create mode 100644 internal/config/gitignore_cache.go create mode 100644 internal/config/gitignore_cache_test.go diff --git a/internal/config/gitignore_cache.go b/internal/config/gitignore_cache.go new file mode 100644 index 000000000..e0e8541f1 --- /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 000000000..a88e776d1 --- /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/manager.go b/internal/config/manager.go index 9b7d467ea..1f104e65d 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 } From 834dd487fff4ca151a0b034695ea59bec156ca8e Mon Sep 17 00:00:00 2001 From: Andrey Kumanyaev Date: Sat, 11 Jul 2026 20:55:29 +0200 Subject: [PATCH 2/7] feat(daemon): standing soft memory limit and post-burst heap release MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The daemon ran with no memory envelope: GOGC defaults, no GOMEMLIMIT, and the only GC tuning scoped to the cold-index window. One allocation burst (warmup passes, an hourly janitor tick, a whole-graph analysis) pinned the process footprint at its peak indefinitely — observed at 16.2 GB on a 16 GiB machine, with the GC then burning 27-77% CPU at idle marking the bloated heap. Boot now installs a standing soft limit — GOMEMLIMIT respected verbatim when set, else GORTEX_DAEMON_MEMLIMIT, else daemon.memory_limit from config, else hostRAM/4 clamped to [1 GiB, 8 GiB] — and the four burst boundaries (warmup complete, janitor ticks that did work, cold-index window close, MCP whole-graph analysis) hand the high-water heap back to the OS via a forced scavenge, logged with the freed byte count. GORTEX_DAEMON_MEMRELEASE=0 disables the releases. The cold-index tuning window captures and restores the standing limit, so the two compose. --- cmd/gortex/daemon.go | 41 ++++- cmd/gortex/daemon_memlimit.go | 271 +++++++++++++++++++++++++++++ cmd/gortex/daemon_memlimit_test.go | 176 +++++++++++++++++++ docs/multi-repo.md | 2 + internal/config/global.go | 16 ++ internal/indexer/gc_tune.go | 37 +++- internal/indexer/gc_tune_test.go | 48 +++++ internal/mcp/server.go | 25 +++ internal/platform/mem_darwin.go | 17 ++ internal/platform/mem_linux.go | 22 +++ internal/platform/mem_other.go | 9 + internal/platform/mem_test.go | 26 +++ 12 files changed, 687 insertions(+), 3 deletions(-) create mode 100644 cmd/gortex/daemon_memlimit.go create mode 100644 cmd/gortex/daemon_memlimit_test.go create mode 100644 internal/platform/mem_darwin.go create mode 100644 internal/platform/mem_linux.go create mode 100644 internal/platform/mem_other.go create mode 100644 internal/platform/mem_test.go diff --git a/cmd/gortex/daemon.go b/cmd/gortex/daemon.go index 4e88b8633..190598b65 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 000000000..5a34ae828 --- /dev/null +++ b/cmd/gortex/daemon_memlimit.go @@ -0,0 +1,271 @@ +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) + if logger != nil { + logger.Info("daemon: released heap to OS", + zap.String("reason", reason), + zap.Duration("elapsed", elapsed), + zap.Int64("freed_bytes", int64(after.HeapReleased-before.HeapReleased)), + 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 000000000..b3ab676af --- /dev/null +++ b/cmd/gortex/daemon_memlimit_test.go @@ -0,0 +1,176 @@ +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) + } + _ = sink + sink = nil + + 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 1364f8939..d825407bd 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/global.go b/internal/config/global.go index c6e79ce08..7d77bc478 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/indexer/gc_tune.go b/internal/indexer/gc_tune.go index 62a31a603..e8e628adb 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 2d012661d..df3a749e4 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,50 @@ 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) + } + _ = sink + sink = nil + + 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/mcp/server.go b/internal/mcp/server.go index a47c97d86..0a624ac2f 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 000000000..c6837e981 --- /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 000000000..393f17a08 --- /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 000000000..024e19993 --- /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 000000000..3609ae219 --- /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) + } + } +} From 86faf6a4d328c9da02d97a21779e85b3e6fb92bf Mon Sep 17 00:00:00 2001 From: Andrey Kumanyaev Date: Sat, 11 Jul 2026 20:55:42 +0200 Subject: [PATCH 3/7] fix(store): bound the symbol-bundle cache by bytes, not entries MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The bundle cache admitted 50,000 entries regardless of size. An entry holds a decoded node plus its full in/out edge lists with meta, so sizes span ~1 KB for a leaf symbol to multiple MB for a hub node — a count cap therefore admitted an unbounded byte footprint on a long-lived daemon. The primary bound is now a byte budget (64 MiB default, GORTEX_BUNDLE_CACHE_MAX_MB override, <= 0 disables the cache) tracked via a deliberately over-counting per-entry estimator, with the count cap kept as a generous secondary guard against floods of tiny entries. Overflow keeps the existing wholesale-clear semantics — entries are cheap to recompute and the cache had no LRU ordering to preserve. --- internal/graph/store_sqlite/bundle_cache.go | 201 ++++++++++++++++-- .../graph/store_sqlite/bundle_cache_test.go | 182 +++++++++++++++- internal/graph/store_sqlite/store.go | 5 +- 3 files changed, 367 insertions(+), 21 deletions(-) diff --git a/internal/graph/store_sqlite/bundle_cache.go b/internal/graph/store_sqlite/bundle_cache.go index 6e3e40a88..055a2d95f 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 6fdf2b2ff..f427fad28 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 e1316c7de..7c2bb2a01 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) From b5150fae257f6f41b276c983d236bc00400b2866 Mon Sep 17 00:00:00 2001 From: Andrey Kumanyaev Date: Sat, 11 Jul 2026 20:55:43 +0200 Subject: [PATCH 4/7] fix(indexer): drop a deleted file's persisted mtime when the watcher evicts it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The watcher's delete/rename patch evicted a file's nodes but left its persisted mtime row behind. Every warm restart then read the row back, found the path gone from disk, and re-ran a scoped reconcile for a file that was already correct — phantom deletions re-processed on every boot. The patch paths (debounced and storm-drain) now prune the mtime after the eviction lands: the in-memory map first, then the store row through the FileMtimeDeleter capability — the delete-side mirror of the add-side rule that a row may exist only while the file's graph state is durable. A delete event for a path still present on disk is downgraded to a modify before this runs, so a revert or atomic-rename replace never loses its row. --- internal/indexer/watcher.go | 31 ++++ internal/indexer/watcher_delete_mtime_test.go | 157 ++++++++++++++++++ 2 files changed, 188 insertions(+) create mode 100644 internal/indexer/watcher_delete_mtime_test.go diff --git a/internal/indexer/watcher.go b/internal/indexer/watcher.go index ded12784a..4acda9d24 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 000000000..47e0a2767 --- /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") +} From f6da4fb6433bf8dfd524742401b3b7e5feed9902 Mon Sep 17 00:00:00 2001 From: Andrey Kumanyaev Date: Sat, 11 Jul 2026 20:55:57 +0200 Subject: [PATCH 5/7] perf(indexer,resolver): scope the global graph passes to the changed-repo set MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The end-of-batch passes re-scanned the whole multi-repo graph whenever at least one file changed — at warmup and again on every hourly janitor tick. Measured on a 26-repo workspace with one changed repo: 503-1601 s of passes, fn-value-callback synthesis alone 264-276 s, and the clone passes materializing every node in the graph (meta included) three times. Node decodes from these sweeps accounted for 107 GB of cumulative allocations in one process lifetime. The passes now consume the changed-repo scope the reconcile batch already arms. A nil scope (fresh full index) is byte-identical to the old whole-graph behavior; with a scope armed: - implements/overrides run their scoped variants over the changed repos' type and interface IDs — a pair survives when either endpoint is in scope, so cross-repo pairs re-derive from the changed side alone - capability, test-edge, and external-call sweeps drive off per-repo edge readers instead of whole-graph kind scans; cross-repo field targets resolve through a lazy per-id fallback instead of a whole-graph map - clone detect/rebuild read GetRepoNodes(prefix) instead of AllNodes(), keeping the whole-graph path only for the empty-prefix single-repo mode - the two heaviest synthesizers, fn-value-callback and value-ref, scan candidates per repo while resolving against the whole graph, so a changed repo's callback still binds to a handler in an unchanged repo value-ref additionally builds its per-file declarator maps only for files that actually carry candidates — replacing a whole-graph scan of the largest node population even on the unscoped path — and the fn-value gate memoizes name lookups for the duration of a pass. --- internal/indexer/capability_edges.go | 145 +++++++++ internal/indexer/clone_incremental.go | 22 +- internal/indexer/clones.go | 23 +- internal/indexer/multi.go | 95 +++++- internal/indexer/scoped_global_passes_test.go | 173 +++++++++++ internal/indexer/test_edges.go | 113 ++++--- internal/resolver/external_calls.go | 42 +++ internal/resolver/fn_value_gate.go | 153 ++++++++-- internal/resolver/framework_synth.go | 51 +++- .../resolver/scoped_global_passes_test.go | 281 ++++++++++++++++++ internal/resolver/value_refs.go | 117 ++++++-- 11 files changed, 1099 insertions(+), 116 deletions(-) create mode 100644 internal/indexer/scoped_global_passes_test.go create mode 100644 internal/resolver/scoped_global_passes_test.go diff --git a/internal/indexer/capability_edges.go b/internal/indexer/capability_edges.go index 864843cc0..6faf25491 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 4f8c47395..6fce556bb 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 f7c485301..8bd5bcea2 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/multi.go b/internal/indexer/multi.go index 54da50058..ce245d797 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 000000000..f9036d60d --- /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 69b1654d1..e10d5812c 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/resolver/external_calls.go b/internal/resolver/external_calls.go index adadce3f5..d58a1280e 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 5392697ba..680880765 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) } @@ -192,7 +238,13 @@ func resolveFnValueName(fileNodes []*graph.Node, name string) string { // (curated-HOF string) function values. Prototype declarations of the name // never make it ambiguous — see uniqueFnValueMatch. func resolveUniqueFnValue(g graph.Store, name string) string { - return uniqueFnValueMatch(g, name, nil) + return resolveUniqueFnValueMemo(g, name, nil) +} + +// 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) } // resolveFnValueCrossModule binds a function value to a uniquely-named @@ -203,7 +255,30 @@ func resolveUniqueFnValue(g graph.Store, name string) string { // 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) + return resolveFnValueCrossModuleMemo(g, name, nil) +} + +// resolveFnValueCrossModuleMemo is resolveFnValueCrossModule with a shared +// per-pass FindNodesByName memo (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 @@ -221,8 +296,14 @@ func resolveFnValueCrossModule(g graph.Store, name string) string { // 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 { + return uniqueFnValueMatchMemo(g, name, exclude, nil) +} + +// uniqueFnValueMatchMemo is uniqueFnValueMatch with a shared per-pass +// FindNodesByName memo (nil disables memoization). +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 +365,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 } @@ -307,6 +394,12 @@ func resolveMemberByType(g graph.Store, typeName, member string) string { // 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 { + return resolveFnValueSelfMemberMemo(g, fromID, member, nil) +} + +// resolveFnValueSelfMemberMemo is resolveFnValueSelfMember with a shared +// per-pass FindNodesByName memo (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 +408,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 ee1771de9..02f674ac0 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 000000000..974170e93 --- /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 861fa9c40..3c6e91df9 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 From 471adbba705c7392d7c9c0f7b4fd25e55c69ea6e Mon Sep 17 00:00:00 2001 From: Andrey Kumanyaev Date: Sat, 11 Jul 2026 21:11:52 +0200 Subject: [PATCH 6/7] fix(daemon): report a non-negative released-bytes delta MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit HeapReleased can shrink between the two reads when concurrent allocation re-acquires released pages faster than the forced scavenge returns them; the raw delta then logs as a negative byte count. Clamp to zero — the honest reading is that nothing was net-released. --- cmd/gortex/daemon_memlimit.go | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/cmd/gortex/daemon_memlimit.go b/cmd/gortex/daemon_memlimit.go index 5a34ae828..7821472b9 100644 --- a/cmd/gortex/daemon_memlimit.go +++ b/cmd/gortex/daemon_memlimit.go @@ -260,11 +260,19 @@ func releaseMemoryToOS(logger *zap.Logger, reason string) { 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", int64(after.HeapReleased-before.HeapReleased)), + zap.Int64("freed_bytes", freed), zap.Uint64("heap_sys_bytes", after.HeapSys), zap.Uint64("heap_released_bytes", after.HeapReleased)) } From f50095fe005894d146183abf9c71e4385258849c Mon Sep 17 00:00:00 2001 From: Andrey Kumanyaev Date: Sat, 11 Jul 2026 22:40:36 +0200 Subject: [PATCH 7/7] fix(lint): drop unused fn-value wrappers and ineffectual sink assignments MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The memoization refactor left the non-memo wrappers resolveFnValueCrossModule / uniqueFnValueMatch / resolveFnValueSelfMember with no callers — the gate now calls the *Memo variants directly. Remove them and fold their doc comments onto the surviving implementations. Replace the '_ = sink; sink = nil' idiom in the two scavenge smoke tests with runtime.KeepAlive(sink): it prevents the write loop from being elided and drops the reference at the same point, without the ineffectual nil assignment ineffassign flags. --- cmd/gortex/daemon_memlimit_test.go | 6 +++-- internal/indexer/gc_tune_test.go | 6 +++-- internal/resolver/fn_value_gate.go | 37 +++++++++--------------------- 3 files changed, 19 insertions(+), 30 deletions(-) diff --git a/cmd/gortex/daemon_memlimit_test.go b/cmd/gortex/daemon_memlimit_test.go index b3ab676af..d4c74b0f2 100644 --- a/cmd/gortex/daemon_memlimit_test.go +++ b/cmd/gortex/daemon_memlimit_test.go @@ -158,8 +158,10 @@ func TestReleaseMemoryToOS_Smoke(t *testing.T) { for i := range sink { sink[i] = byte(i) } - _ = sink - sink = nil + // 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) diff --git a/internal/indexer/gc_tune_test.go b/internal/indexer/gc_tune_test.go index df3a749e4..b5e49a6b9 100644 --- a/internal/indexer/gc_tune_test.go +++ b/internal/indexer/gc_tune_test.go @@ -538,8 +538,10 @@ func TestFreeOSMemoryAfterColdIndex(t *testing.T) { for i := range sink { sink[i] = byte(i) } - _ = sink - sink = nil + // 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) diff --git a/internal/resolver/fn_value_gate.go b/internal/resolver/fn_value_gate.go index 680880765..39fabcd52 100644 --- a/internal/resolver/fn_value_gate.go +++ b/internal/resolver/fn_value_gate.go @@ -236,7 +236,7 @@ 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 resolveUniqueFnValueMemo(g, name, nil) } @@ -247,19 +247,14 @@ func resolveUniqueFnValueMemo(g graph.Store, name string, memo map[string][]*gra return uniqueFnValueMatchMemo(g, name, nil, memo) } -// resolveFnValueCrossModule binds a function value to a uniquely-named +// 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 resolveFnValueCrossModuleMemo(g, name, nil) -} - -// resolveFnValueCrossModuleMemo is resolveFnValueCrossModule with a shared -// per-pass FindNodesByName memo (nil disables memoization). +// 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) } @@ -281,8 +276,9 @@ func findNodesByNameMemo(g graph.Store, name string, memo map[string][]*graph.No 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 @@ -295,12 +291,6 @@ func findNodesByNameMemo(g graph.Store, name string, memo map[string][]*graph.No // 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 { - return uniqueFnValueMatchMemo(g, name, exclude, nil) -} - -// uniqueFnValueMatchMemo is uniqueFnValueMatch with a shared per-pass -// FindNodesByName memo (nil disables memoization). func uniqueFnValueMatchMemo(g graph.Store, name string, exclude func(*graph.Node) bool, memo map[string][]*graph.Node) string { def, proto := "", "" for _, n := range findNodesByNameMemo(g, name, memo) { @@ -390,15 +380,10 @@ func resolveMemberByTypeMemo(g graph.Store, typeName, member string, memo map[st 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 { - return resolveFnValueSelfMemberMemo(g, fromID, member, nil) -} - -// resolveFnValueSelfMemberMemo is resolveFnValueSelfMember with a shared -// per-pass FindNodesByName memo (nil disables memoization). +// 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 {