Skip to content

perf: bound indexing memory and flush partial progress on interrupt - #159

Open
iggy wants to merge 1 commit into
masterfrom
perf/bound-indexing-memory
Open

perf: bound indexing memory and flush partial progress on interrupt#159
iggy wants to merge 1 commit into
masterfrom
perf/bound-indexing-memory

Conversation

@iggy

@iggy iggy commented Aug 16, 2026

Copy link
Copy Markdown
Owner

Problem

gocate -updatedb was slow and memory-hungry on large trees. Profiling showed the cost was not hashing — it was the store write path:

  • Every Upsert ran SELECT ... FirstRow() with no usable index → a full table scan per file (O(n²) total). modernc.org/ql ignores the index for FirstRow (verified: 136 ms/row at 200k rows, 1.0× with index).
  • hashFile called os.ReadFile, so peak memory = file size.
  • Each row committed in its own BEGIN TRANSACTION; … COMMIT;.

Result: on a 32-core box it sat at ~120% CPU because the hash workers finished fast but funneled into one consumer doing slow sequential scans.

Changes

  • internal/index/hash.go — stream instead of slurp: imohash via SumSectionReader (fixed samples) and xxh3 via io.Copy into xxh3.New(). Peak memory is now bounded by the read buffer. The SectionReader is rewound between passes (it's left at the tail after sampling). Hashes are byte-identical to before, so existing DB rows stay valid.
  • internal/index/index.goRunCtx loads the host's rows once into a read-only snapshot shared by the walk (quick-skip) and consumer (insert-vs-update) with no locking, eliminating the per-row SELECTs. The consumer batches writes into single transactions (default 1000 rows / 4 GiB). The walk bounds concurrency by both file count (Workers) and bytes in flight (MaxBytes, default 256 MiB/worker) so a few huge files can't starve small-file parallelism.
  • cmd/gocate/main.go — cancels the run's context on SIGINT/SIGTERM so the consumer flushes its in-flight batch (committing partial progress) before close; a second SIGINT exits 130.
  • internal/store/store.goLoadExisting (hostname-scoped) + WriteBatch (one transaction for many rows, rollback on error). Upsert reuses the batch path.

Tests

  • TestHashFileMatchesReadFileBasis — proves streaming hashes equal the old os.ReadFile approach across small/medium/large files.
  • TestLoadExistingScopesByHost — proves a multi-host DB can't leak cross-host rows into the path-keyed snapshot.
  • All existing tests pass; go vet clean.

Verification (5.2 GB, 4530 files)

wall CPU peak RSS
before 9.0 s 136% ~18 MB
after 3.7 s ~110% ~18 MB

And a SIGINT mid-run commits partial progress (verified: 20k rows after interrupt, DB reopens cleanly).

@iggy
iggy force-pushed the perf/bound-indexing-memory branch from 4f0c4f0 to e3d6566 Compare August 16, 2026 22:25
Indexing was O(n^2) in wall time and peaked at file-size memory:
each Upsert ran a full-table-scan SELECT (no index, ql ignores it for
FirstRow) and hashFile called os.ReadFile, loading the whole file.

Changes:
- hashFile now streams: imohash via SumSectionReader (fixed samples)
  and xxh3 via io.Copy into xxh3.New(). Peak memory is bounded by the
  read buffer, not file size. Rewind the SectionReader between passes
  (SumSectionReader leaves it at the tail). Hashes are byte-identical
  to before (TestHashFileMatchesReadFileBasis), so DB rows stay valid.
- index.RunCtx loads the host's rows once (LoadExisting) into a
  read-only snapshot shared by the walk (quick-skip) and consumer
  (insert-vs-update) without locking, eliminating per-row SELECTs.
- Consumer batches writes into single transactions (BatchRows default
  1000 / BatchBytes default 4 GiB): ~1000x faster than per-row commits,
  and a Ctrl-C forfeits at most one in-flight batch.
- Walk bounds concurrency by both file count (Workers) and bytes in
  flight (MaxBytes, default 256 MiB/worker) so a few huge files cannot
  starve small-file parallelism.
- main.run cancels the run's context on SIGINT/SIGTERM so the consumer
  flushes its in-flight batch (committing partial progress) before
  closing the store; a second SIGINT exits 130 immediately.
- LoadExisting scopes by hostname so a multi-host DB cannot leak
  cross-host rows into the path-keyed snapshot.
@iggy
iggy force-pushed the perf/bound-indexing-memory branch from e3d6566 to 81bcb88 Compare August 16, 2026 22:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant