perf(invertedindex): Tier-1 in-place allocation / build-RSS reductions (byte-identical) - #110
Merged
Merged
Conversation
…in+[]byte copy) encodeForwardValue built the forward-map value as []byte(strings.Join(kw, "|")), which allocates twice (the joined string, then the []byte copy) and copies the whole multi-KB value once per indexed document. Hand-roll the join into a single pre-sized make([]byte,0,n) buffer: one allocation, no intermediate string. Byte-for-byte identical output (guarded by TestEncodeForwardValue_ByteIdentity: hand-written literals AND strings.Join parity for multi / single / empty / single-empty / interior-empty / '|'-in-keyword). The allocation reduction is pinned by TestEncodeForwardValue_SingleAlloc (min-of-trials AllocsPerRun, robust to upward MemStats contamination from background goroutines). No on-disk format change, no reindex. Generated with [Claude Code](https://claude.ai/code) via [Happy](https://happy.engineering) Co-Authored-By: Claude <noreply@anthropic.com> Co-Authored-By: Happy <yesreply@happy.engineering>
…emoveDuplicates map) Every posting-row write ran removeDuplicatesEfficiently (a fresh map[int64]bool + slice + full hash pass) and THEN encodeInvertedValue, which already sorts — so the separate dedup map was redundant. Two of writeInvertedIndex's three callers (rewriteIndex, removeDocumentsFromInvertedIndex) even pass ids already drained from a map, making that dedup 100% wasted. Fold set-dedup into the encoder: slices.Compact after the existing slices.Sort (adjacent-equal removal == set-dedup post-sort), and drop the removeDuplicatesEfficiently call in writeInvertedIndex (the flush path, which intentionally carries duplicates, is now deduped by the encoder). Also replace the per-docid PutUvarint+tmp+append with binary.AppendUvarint (one less copy). On-disk bytes are byte-for-byte identical: old (dedup -> sort -> delta) and new (sort -> compact -> delta) both emit the delta-varint of the sorted-unique set. Pinned by TestEncodeInvertedValueGoldenBytes (hand-typed literals) and TestEncodeInvertedValueRoundTripFullIntRange (independent oracle, full int64 range). removeDuplicatesEfficiently retained, covered by its own tests. No reindex. Generated with [Claude Code](https://claude.ai/code) via [Happy](https://happy.engineering) Co-Authored-By: Claude <noreply@anthropic.com> Co-Authored-By: Happy <yesreply@happy.engineering>
… key encodeInvertedKey read time.Now().UnixMicro() on every key it built — ~10.6M+ vDSO clock reads over a full build (one per flushed row, per merge rewrite, per delete rewrite). The tick is opaque to decodeInvertedKey and plays no role in uniqueness; idx.keySeq.Add(1) is the sole guarantor that keys are distinct. Thread the tick in as a parameter, sampled once per batch/rewrite: - flushPendingWrites captures now := time.Now() after the cooldown early-return (reused for lastFlushWriteTime, and later by the I2 age check) and passes now.UnixMicro() — one clock read for the whole flush (the dominant build path); - rewriteIndex and removeDocumentsFromInvertedIndex each sample the tick once before their inner re-encode loop (signatures unchanged). Key grammar <prefix><doccount>|<tick>.<seq> is unchanged (tick still decimal micros, seq still keySeq) so decode and existing data are unaffected — no reindex. TestEncodeInvertedKey_UniqueUnderIdenticalTick pins that a constant tick still yields distinct keys (via keySeq) and that the passed tick is actually embedded. Generated with [Claude Code](https://claude.ai/code) via [Happy](https://happy.engineering) Co-Authored-By: Claude <noreply@anthropic.com> Co-Authored-By: Happy <yesreply@happy.engineering>
…-> int64 nanos)
The per-keyword pending-write/-delete caches hold a relatedDocs{DocIds; UpdatedAt}
per in-flight keyword — up to ~10.6M entries co-resident in the unbounded build
buffer. UpdatedAt was a 24-byte time.Time embedding a GC-scanned *Location
pointer, and its only use is the soft young-entry flush-age skip.
Change UpdatedAt to an int64 unix-nanos timestamp: the struct drops 48B -> 32B and
loses the pointer (~16B x live-term-count less peak RSS + ~10.6M fewer GC-scanned
pointers on the priority-#2 memory axis). updateIndex/removeIndex set
time.Now().UnixNano(); the flush functions compute nowNanos from the clock read
they already do past the cooldown guard (the write side reuses the tick's now)
and compare time.Duration(nowNanos - UpdatedAt), each keeping its own timeout
selector (flushWaitTimeout 3s / flushDeleteWaitTimeout 5s).
UpdatedAt is never serialized, so no on-disk change. The heuristic moves from a
monotonic to a wall clock; forced/closing/pressure flushes drain regardless, so a
clock step only shifts a soft skip by one cycle (documented on the field). Four
symmetric young-survives / aged-drains tests (write + delete) pin the age math's
sign and nanosecond scale on both previously-uncovered skip branches; the young
tests use a 1h timeout to stay deterministic while still catching a .Unix() scale
regression (which yields a >1h delta).
Generated with [Claude Code](https://claude.ai/code)
via [Happy](https://happy.engineering)
Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Happy <yesreply@happy.engineering>
…bble pool
The four batch sites (flushPendingWrites, flushPendingDeletes, mergeKeywordsIndex,
DeleteTable) Commit their batch but never Close it. For PebbleBatch, Commit only
applies; Close is what returns the *pebble.Batch and its grown data buffer to
pebble's pool — otherwise each batch is left for GC.
Add `defer func() { _ = batch.Close() }()` right after each batch creation
(exactly one per site, no double-close; DeleteTable keeps `return batch.Commit()`
so its Commit error is still returned — the deferred Close runs after the return
value is evaluated). Close-after-Commit is contract-safe on kv.Batch.
TestBatchSitesCloseCommittedBatch drives all four sites through a store-level
Close recorder and asserts each Closes its committed batch (count 0 before, >= 4
after). No on-disk change.
Generated with [Claude Code](https://claude.ai/code)
via [Happy](https://happy.engineering)
Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Happy <yesreply@happy.engineering>
Collaborator
Author
Benchmark — real linux corpus, prod defaultMeasured
Every iteration agreed on direction (not noise). Search is untouched, and the identical hit count on the full corpus confirms byte/semantic correctness at scale. The −14% disk is an incidental bonus of the per-batch tick change: one flush's rows now share a Net: a solid build-axis win (allocator traffic −20%, build −6.6%, disk −14%, peak mem −4–6%) with zero search regression — squarely on the |
codetrek
approved these changes
Jul 10, 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
2 participants
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.
What
Five small, in-place, byte-identical-on-disk changes to
core/invertedindexthat cut allocator/GC churn and build-phase peak RSS — the top-priority axes (build/ingest ≫ memory > search). No on-disk format change, no reindex.Sourced from a multi-agent audit of the package's hot paths (26 findings → 15 distinct, each adversarially verified); this PR is the "Tier 1" bundle — the CONFIRMED, zero-risk, top-axis wins. One commit per item:
c196692[]byte(strings.Join(...))= 2 allocs + full copy per document)1111c2dslices.Compactafter the existing sort) +binary.AppendUvarint; drop the redundant per-rowremoveDuplicatesEfficientlymap (100% wasted on the merge/delete callers)24d774dtime.Now().UnixMicro()per key (~10.6M+ vDSO reads/build → ~one per flush);keySeqstill the sole uniqueness guarantor07b0230time.Time→int64unix-nanos: 48B→32B per in-flight keyword, drops a GC-scanned*Locationpointer (~10.6M fewer on a full build)6df3e30Why / honest framing
None of these is a wall-clock game-changer on its own (pebble commit + per-posting append/hash dominate build; per-file I/O dominates search). The value is a clean reduction of allocator/GC churn and peak build RSS — which is exactly what matters for the low-RAM deployment target where memory outranks search latency. All byte-identical on disk (proven by golden tests), so they ship together with zero migration risk.
Correctness / invariants
encodeForwardValue,encodeInvertedValue) + the unchanged decode round-trip suites.encodeInvertedKeypreserves the key grammar (tick is opaque;keySeqguarantees uniqueness under an identical tick — tested).relatedDocs.UpdatedAtis never serialized; the flush-age heuristic moves monotonic→wall clock (soft skip only; forced/closing/pressure flushes drain regardless). Symmetric young-survives / aged-drains tests (write + delete) pin the age math's sign and nanosecond scale.defer Close()is contract-safe onkv.Batch(Commit doesn't release; Close returns the batch to pebble's pool). Exactly one Close per site (no double-close); a store-level Close recorder test guards all four sites.Verification
Every item built via strict TDD (real red → green) with a multi-agent review loop per item. Full gates green with
go1.24.2(CI parity):build ./...,test ./invertedindex/ ./engine/,test -race ./invertedindex/,gofmt,vet, and go-cov zero CRITICAL (all touched functions ≥80%; thewriteInvertedIndexpackage-var seam is an EFFECT=0 tolerated range).git diff maintouches onlycore/invertedindex/*.go.🤖 Generated with Claude Code