Add memory cap to indexer to prevent OOM on large monorepos - #87
Merged
Conversation
The background indexer (tgrep serve) now checks the process RSS after each batch and stops indexing early when it exceeds a configurable memory budget. This prevents the host from being OOM-killed on very large monorepos that produce an unbounded in-memory trigram overlay. Changes: - Add --max-memory <MB> flag to 'tgrep serve' (defaults to 50% of physical RAM, clamped between 512 MB and 16 GB) - Add cross-platform mem.rs module (Windows/Linux/macOS) for querying process RSS and total physical memory - When the cap is hit, indexing stops and the on-disk index is marked as partial (complete=false) so subsequent starts will continue indexing the remaining files incrementally Fixes: github/copilot-cli#3976 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Replace the 'stop at the memory cap (partial index)' behavior with a memory-bounded flush-and-continue strategy so the bulk indexer keeps peak memory under the budget AND still produces a COMPLETE index. The naive approach -- reuse the existing flush_index_to_disk mid-build -- does not bound peak memory, because HybridIndex::full_snapshot() rebuilds the entire merged inverted index in heap (O(total index size)). Key insight: during the initial bulk build the live overlay is strictly append-only (the file watcher and auto-save are both suppressed while indexing==true), so a flush can copy the existing on-disk postings verbatim from the reader's mmap and only the bounded overlay needs to live in heap. Changes: - tgrep-core: add builder::append_overlay_to_index -- a streaming 2-way append-merge of the reader's sorted lookup table (read from mmap, posting bytes copied verbatim) with the live overlay's sorted postings. Peak heap stays bounded to the overlay snapshot, independent of total index size. - tgrep-core: add IndexReader::nth_trigram_raw (zero-copy access to a trigram's raw posting bytes) and HybridIndex::reader_arc. - serve.rs: in background_index_build, when RSS exceeds the cap, perform an incremental_flush (publish complete=false) and clear the overlay, then continue. The final flush publishes complete=true. Extract the shared move+reopen+swap+prune logic into publish_staged_index, reused by flush_index_to_disk and incremental_flush. - main.rs: update --max-memory help to reflect flush-and-continue. Tests: - core unit test for append-merge correctness (file IDs, sorted postings, base-mask preservation, overlay sentinel masks). - CLI integration test: serve with --max-memory 1 over 1200 files forces multiple incremental flushes; asserts meta complete=true, all files present, and the last-indexed file is searchable. Fixes: github/copilot-cli#3976 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The initial bulk build's CPU-heavy work (file reads + trigram extraction) ran via rayon across every logical core, saturating the host. Confine it to a bounded worker pool so tgrep stays a good neighbor — especially when embedded in another tool. - Add --max-cpu <PERCENT> to 'tgrep serve' (percentage of logical cores, default 50%). - New cpu.rs: index_thread_count(percent) maps the budget to a worker count clamped to 1..=cores. - background_index_build runs the parallel extraction par_iter inside a dedicated rayon ThreadPool sized to the budget (pool.install), falling back to the global pool if one can't be built. The single-threaded merge/flush paths need no pooling. Tests: cpu:: unit tests for clamping/bounds; the memory_cap integration test exercises the bounded pool end-to-end at the default 50%. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Review found two correctness issues in the resource-cap path: - The indexing-to-flushing handoff relied on relaxed atomics even though the auto-save loop must not observe both flags as false. Use sequentially consistent accesses for the indexing/flushing coordination flags. - The final bulk-build publish still used the full_snapshot flush path, which materializes the entire reader+overlay inverted index in heap and defeats the memory cap. Reuse the append-only streaming flush for the final complete publish as well, including staged filestamps. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds memory- and CPU-bounding to the initial bulk indexing path so large monorepos can be indexed without OOMing or saturating the host, by introducing a streaming append-only flush strategy and limiting rayon parallelism.
Changes:
- Implemented a streaming “append overlay onto existing on-disk index” merge to avoid materializing reader postings on the heap during bulk-build flushes.
- Added a configurable
--max-memory(MB) cap and--max-cpu(% of cores) cap fortgrep serve, with runtime enforcement during background indexing. - Added an integration test that forces repeated mid-build flushes and asserts the final index is complete and searchable.
Show a summary per file
| File | Description |
|---|---|
| tgrep-core/src/reader.rs | Adds a zero-copy API to read the i-th trigram’s raw posting bytes from the mmap for streaming merges. |
| tgrep-core/src/hybrid.rs | Exposes an Arc<IndexReader> snapshot to support memory-bounded flush flows. |
| tgrep-core/src/builder.rs | Implements the streaming append-only merge writer and adds a unit test for merge correctness. |
| tgrep-cli/tests/memory_cap.rs | New integration test that forces flushes under a tiny memory cap and validates completeness/searchability. |
| tgrep-cli/src/serve.rs | Adds memory-cap-triggered incremental flushes, final flush via append-only path, and CPU-limited indexing pool; strengthens atomic coordination. |
| tgrep-cli/src/mem.rs | New cross-platform RSS/physical-memory helpers plus default cap computation. |
| tgrep-cli/src/main.rs | Adds CLI flags --max-memory and --max-cpu and wires them into serve::run. |
| tgrep-cli/src/cpu.rs | Computes indexing thread count from a CPU budget percentage with tests. |
| tgrep-cli/Cargo.toml | Adds platform-specific deps (windows-sys, libc) needed for memory introspection. |
| Cargo.lock | Locks the new dependencies. |
Review details
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 9/10 changed files
- Comments generated: 3
- Review effort level: Low
…urrent RSS on macOS - builder: advance past truncated/unreadable reader trigrams during the append-overlay merge so remaining reader entries are never dropped - reader: use checked_mul for posting byte length to avoid 32-bit/corrupt overflow - mem: report current resident size on macOS via proc_pidinfo instead of peak ru_maxrss Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
sysctl expects a mutable name pointer; use a mut mib array with as_mut_ptr. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…uption - builder: fail the flush (IndexCorrupted) on an unreadable in-range reader entry instead of silently dropping trigrams; serve keeps previous reader + live overlay as fallback - builder: reject >u32::MAX reader files, and use checked add + try_from for posting-list length and overlay file-id math - main: reject --max-memory 0 via clap range and use saturating_mul when converting MB to bytes Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The final bulk-build handoff set flushing=true/indexing=false and only then let flush_append_only_overlay acquire snapshot_gate. handle_fs_event only skips while indexing is true, so a filesystem event could slip into that gap, take snapshot_gate.read() first, and mutate the LiveIndex before the final snapshot. If it touched a path already present in the on-disk reader, append_overlay_to_index would append a duplicate/stale path, breaking its brand-new-paths precondition and corrupting the persisted index. Acquire snapshot_gate.write() *before* clearing indexing and hold it across the whole final flush by splitting the flush into flush_append_only_overlay (acquires the gate) and flush_append_only_overlay_locked (assumes it is held). Any watcher event that observes indexing=false now blocks on the gate until the publish completes and then applies to the newly published reader, so no event is lost. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…xture - reader: compute nth_trigram_raw range in u64 and bounds-check against the mmap length before converting to usize, so a large/corrupt offset can't truncate on 32-bit targets and slice the wrong region - tests: reduce memory_cap fixture from 1200 to 700 files (still exceeds the 500 batch size and forces multiple flushes) to keep CI fast Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Explicitly reject start > len (not just end > len) so a corrupt zero-length entry with an out-of-range offset can never slice past the mmap. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The Windows RSS query zeroed the struct but left cb at 0. GetProcessMemoryInfo requires cb to hold the struct size on input; without it the call can fail, silently disabling the memory cap on Windows. Set cb before the call and add mem-query regression tests asserting non-zero RSS/physical memory. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
msftsiwei
approved these changes
Jul 1, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds a configurable memory cap to the indexer to prevent OOM when building indexes on large monorepos, plus CPU usage limiting during the initial index build.
Changes
tgrep-core/src/builder.rs,tgrep-cli/src/mem.rs): Bound peak indexing memory via a flush-and-continue strategy so the full index is still produced without exhausting RAM.tgrep-cli/src/serve.rs): Coordinate memory-bounded bulk index flushes.tgrep-cli/src/cpu.rs): Limit CPU usage during the initial index build.tgrep-cli/tests/memory_cap.rs): Coverage for the memory cap behavior.Testing
memory_cap.rs.