perf: cut index and query hot paths (ASCII decode, fingerprint skip, query cache) - #1401
Closed
HaD0Yun wants to merge 3 commits into
Closed
perf: cut index and query hot paths (ASCII decode, fingerprint skip, query cache)#1401HaD0Yun wants to merge 3 commits into
HaD0Yun wants to merge 3 commits into
Conversation
chardet.detect() accounts for ~98% of per-file parse cost (672us of a 686us budget on a 4KB document), and it runs even when the buffer is plain ASCII and every candidate encoding would decode identically. Skip detection when the buffer is pure ASCII (all bytes 0x01-0x7F, no ESC, at least 4 bytes long). For those inputs chardet can only return ASCII, UTF-8, ISO-8859-x, Shift_JIS, Big5, EUC-JP, EUC-KR or GB18030: UTF-16/32 need a BOM or NUL bytes, ISO-2022 needs ESC, and the windows-125x families need C1 bytes. Every one of those decodes 0x01-0x7F to the same code points, and NFC is the identity there, so the output is provably unchanged. Buffers shorter than 4 bytes keep the legacy path: chardet misreads them as UTF-32LE and iconv-lite drops the incomplete unit, so the existing output for 1-3 byte control-character buffers is the empty string. Preserving that keeps this a pure optimization. Cold refresh of a 200-document ASCII corpus goes from 175ms to 42ms (4.2x, median of 5). Korean corpora are unchanged at ~1.0x because the fast path does not apply to them. Equivalence is covered by a test that keeps a copy of the previous implementation and compares both across boundary lengths, boundary byte values, BOM, NUL bytes, combining characters and pseudo-random buffers.
A no-op refresh rewrote the whole BM25 index and re-read every mirror, even when nothing on disk had changed. BM25 was 61% of that refresh. Record a sha256 of the normalized mirror content on each parsed entry (the string is already in memory, so this costs no extra read) and commit a fingerprint sidecar after the artifact. A refresh whose fingerprint matches the stored one reuses the artifact instead of rebuilding it. Legacy entries without a digest are backfilled once. Skipping only stays correct if the artifact and its fingerprint cannot interleave across processes, so this also adds a cross-process refresh lock. The CLI builds a fresh agent per command, so 'autorag watch' in one terminal and 'autorag refresh' in another are two processes sharing one index directory; without a lock on disk they can commit a fingerprint that describes a newer corpus than the artifact it points at. The lock wraps the existing acquireFileLock primitive and lives at .autorag/refresh.lock, outside every directory 'index reset' removes. 'index reset' and 'index rebuild' take the same lock. Because the fingerprint decides whether a stored artifact is trusted, the code that determines artifact meaning is pinned by source-region markers that a test digests: chunking, chunk id hashing, fallback persistence schema, tantivy schema construction, the fingerprint material itself, and mirror normalization. Changing any of them fails the test until INDEX_SEMANTICS_VERSION is bumped or the recorded digest is refreshed deliberately. Query-time code (tokenize, bm25Score, k1, b) is deliberately outside those regions: it never invalidates a stored artifact, so guarding it would force needless global rebuilds. Behavior change: a refresh refused because another one holds the lock now exits 1 with ok:false and outcome:"busy" instead of exiting 0. Automation that only checks the exit code previously read a refused refresh as a successful one. Watch ticks are unaffected; busy is normal backpressure there. No-op refresh on a 2000-document corpus goes from 78ms to 44ms.
The fallback engine re-read and re-tokenized the whole index on every query, then walked every chunk to compute document frequencies. Cost was driven by corpus size alone: a query matching two documents cost the same as one matching all of them, because the scan was linear rather than an inverted lookup. Cache the prepared state (tokenized chunks plus postings) keyed by the fingerprint sidecar the previous commit introduced, derive document frequencies from postings, and narrow candidates to chunks that actually contain a query term. Narrowing cannot change the output: bm25Score skips terms with zero frequency and its idf is positive for every df <= N, so a chunk without any query term scores exactly zero and the existing score > 0 filter already dropped it. Scoped document frequencies still count only postings inside the scope, so scoping semantics are unchanged. The tantivy path opened the index and built a searcher per query; that handle is now cached under the same key. The cache is only held when the fingerprint sidecar exists, since that is the cross-process invalidation key; without it the prepared state is used for the current query and discarded. Retention is bounded by an estimate of the bytes the structure pins, measured against real heap growth. It is an empirical budget for the fallback structure, not a hard process memory cap, and it does not cover native tantivy handles. A selective query on a 1,200-chunk corpus goes from 19.0ms to 0.021ms, and growing the corpus 8x no longer grows selective query cost. Equivalence is covered by tests comparing warm and cold instances across both engines, every scope, and mutated corpora.
HaD0Yun
marked this pull request as ready for review
August 8, 2026 14:57
Collaborator
Author
|
Closing in favour of a cleaner split — this was one 4,700-line PR from a stranger touching four separate concerns, which is not a fair thing to hand a reviewer.
No code was dropped; the same three commits are redistributed. Sorry for the noise. |
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.
What
Three hot-path optimizations, measured against this branch's merge base with a separate checkout of
mainon the same machine.maincold refreshmainThe Korean row is the honest headline caveat: the parser fast path only applies to pure-ASCII sources, so a Korean-document corpus sees no cold-refresh gain. It still benefits from the index and query work (no-op refresh 2.0x). Given that this project targets Korean documents, please weigh that when scoping.
Commits
Three independent changes, split so they can be reviewed or landed separately. Happy to split into separate PRs on request — the natural boundary is
parseralone, andfingerprint skip + lockbeforequery cache(the cache uses the fingerprint sidecar as its invalidation key).perf(parser)— skipchardet.detect()for pure-ASCII buffers. Independent of the other two.perf(retrieval)fingerprint skip + cross-process lock — a no-op refresh no longer rebuilds the BM25 index. The lock is not optional here: skipping is only sound if artifact and fingerprint commits cannot interleave across processes.perf(retrieval)query cache — the fallback engine no longer re-tokenizes the corpus and linearly scans every chunk per query.Behavior changes
These are deliberate and observable. Please push back if you'd prefer different semantics.
autorag refreshrefused because another refresh holds the lock now exits 1 (ok:false,outcome:"busy") instead of exiting 0. Previously a refused refresh looked successful to anything checking only the exit code. This can break existingset -eCI or cron that treated contention as success; conversely, exit-code-based retry can now detect and back off.index reset/index rebuildexit 1 when refused for the same reason, and reset removes nothing in that case..autorag/refresh.lockis now an explicit error, not a "busy" result. Previously it silently made refresh and reset do nothing.watchis unaffected — busy is normal backpressure there and ticks still exit 0.Correctness
The parser change is the one that touches stored content, so it is argued rather than sampled: for a pure-ASCII buffer the set of encodings
chardetcan return is enumerable, and every member decodes0x01-0x7Fidentically. Sampling backs it up — a test keeps a copy of the previous implementation and compares both across boundary lengths, boundary byte values, BOM, NUL bytes, combining characters and pseudo-random buffers.Because the fingerprint decides whether a stored artifact is trusted, the code that determines artifact meaning is pinned by source-region markers that a test digests: chunking, chunk id hashing, fallback persistence schema, tantivy schema construction, the fingerprint material itself, and mirror normalization. Changing any of them fails until
INDEX_SEMANTICS_VERSIONis bumped or the recorded digest is refreshed deliberately.This guard is a manual allowlist. It does not discover new files or helpers on its own. If a future change puts artifact-shaping logic outside the marked regions, the guard will not notice.
Query-time code (
tokenize,bm25Score,k1,b) is intentionally outside those regions. Changing the tokenizer changes query results immediately and correctly against the stored artifact — verified by a test that mutates it and observes the artifact mtime stay unchanged while results change. Guarding it would force needless global rebuilds.Known limitations
chardeteven for valid UTF-8, so short or ambiguous inputs could decode differently. Doing it would be a semantics change requiring a version bump and migration.index rebuildis concurrency-atomic but not rollback-atomic. Delete and rebuild are covered by one lock, so no other refresh can interleave. But if parsing or building fails after the delete, the indexes stay removed and you must rerun.artifactBytesonly checks size. A same-length tampered artifact passes. It is truncation detection, not integrity verification.autoengine sticks to fallback after one tantivy failure, because the engine is part of the fingerprint. Results stay correct but quality silently stays lower untilindex rebuild.--force. An unchanged source with the same mtime/size is not re-mirrored, so a new parser or achardet/iconv-liteupgrade does not re-parse existing documents.mkdiratomicity is unverified on NFS.Verification
biome check(252 files, 0 warnings),tsc --noEmit, andbun run buildall pass.vitest runreports 3 failures out of 1,365:test/parser/parser.test.tstest/agent/parser-mirror.test.tstest/jikji/installer.test.tsThat last one is worth flagging separately: the suite is load-dependent regardless of this branch. Running
vitest runon the unmodified merge base on this machine produced more failures (12 files / 15 tests) than this branch (10 files / 13 tests) in the same session.vitest.config.tssets no pool or concurrency limits, so wall-clock-budgeted tests compete under load. Happy to open that as a separate issue.Test count 1,249 → 1,365. New tests cover fingerprint skip, artifact integrity, semantics guard, mirror sync contracts, query cache equivalence and bounds, lock behavior in-process and across real OS processes, and decode equivalence.
Not included
An earlier revision parallelized mirror parsing. It was reverted before this PR: measured gain was 1.01–1.10x on markdown (parsing is dominated by synchronous CPU, not I/O wait), and adversarial testing found a deadlock on a zero concurrency limit, unbounded fan-out on NaN, and a checkpoint/disk digest mismatch that could serve stale results indefinitely. It needs a deterministic commit-prefix design and real PDF/Office corpus measurements, so it belongs in its own change.