perf: fuzzy/regexp candidate-collection optimizations#2382
Draft
capemox wants to merge 2 commits into
Draft
Conversation
Three standalone optimizations in the fuzzy candidate-collection path (search/searcher/search_fuzzy.go), none of which change results: - findFuzzyCandidateTerms kept a termSet map and did a map insert per candidate. But the dictionary iterator already yields each term once (merged across segments), so the map is only needed to de-duplicate synonym terms against dictionary terms. Skip the map entirely when the field has no synonyms (the common case) — the non-automaton fallback path already appends without a map, confirming this. - prefixTerm was built by concatenating runes one at a time (O(n^2), realloc per rune). Replace with a behavior-preserving slice of the original string at the first rune boundary at/after prefix. - Hoist the repeated ctx.Value(FuzzyMatchPhraseKey) lookups in NewFuzzySearcher into a single lookup. Benchmark (BenchmarkFuzzyCandidateCollection, added here: scorch index, 1000 terms / 3000 docs, fuzziness-2 query matching 280 candidates), parent vs this commit, Apple M4 Pro, count=12 via benchstat: sec/op 144.0µ -> 135.5µ -5.9% (p=0.000) B/op 693.3Ki -> 667.1Ki -3.8% (p=0.000) allocs/op 988 -> 975 -1.3% (p=0.000) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Fuzzy and regexp candidate collection iterate the field dictionary via the levenshtein/regexp automaton and use only the term and edit distance — DictEntry.Count is discarded. The segment iterator nonetheless read each visited term's postings list to compute that count. Add a termDictionaryOmitCount optional interface (AutomatonIteratorOmitCount) and use it from the fuzzy and regexp field-dict constructors so the count (and its per-term postings read) is skipped. This is opt-in per call site: FieldDict / FieldDictPrefix / FieldDictRange still populate counts, since faceting and public dictionary APIs rely on them. The wiring uses a type assertion, so it degrades gracefully: with a zapx that predates AutomatonIteratorOmitCount it falls back to the count-reading path (TestFieldDictFuzzyAutomatonOmitsCount, added here, skips in that case). The optimization activates once the zapx/v17 dependency is bumped to a release that includes the omit-count iterator. Benchmark (BenchmarkFuzzyCandidateCollection from the previous commit; scorch index, 1000 terms / 3000 docs, fuzziness-2 query matching 280 candidates), parent vs this commit, Apple M4 Pro, count=12 via benchstat: sec/op 135.5µ -> 122.4µ -9.7% (p=0.000) B/op 667.1Ki -> 659.9Ki -1.1% (p=0.000) allocs/op 975 -> 691 -29.1% (p=0.000) Combined with the previous commit: -15.0% sec/op, -30.1% allocs/op vs the original candidate-collection path. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
coverage: 52.295% (-0.03%) from 52.321% — perf-fuzzy-regexp-opts into master |
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.
Split out from #2381 per review — the fuzzy/regexp candidate-collection optimizations, as their own PR. No functional/API changes.
Changes
search/searcher/search_fuzzy.go): skip the per-candidate dedup map when the field has no synonyms (the dictionary iterator already yields each term once; the map only exists to de-dup synonym terms), replace the O(n²)prefixTermrune-concat with a zero-alloc slice, and hoist repeatedctx.Valuelookups.index/scorch/snapshot_index*.go): fuzzy/regexp collectors use only the term + edit distance and discardDictEntry.Count, yet the segment iterator read each candidate's postings list to compute it. A new opt-inAutomatonIteratorOmitCountpath skips that read, wired via a graceful type-assertion fallback.FieldDict/FieldDictPrefix/FieldDictRangestill populate counts.Depends on blevesearch/zapx#436 — the omit-count optimization activates once the
zapx/v17dependency is bumped to a release that includesAutomatonIteratorOmitCount. Until then the wiring falls back to the count-reading path (the wiring test skips), so this is safe to merge independently.Benchmark
BenchmarkFuzzyCandidateCollection(scorch index, 1000 terms / 3000 docs, fuzziness-2 query matching 280 candidates), original → both commits, Apple M4 Pro, count=12 via benchstat:All p=0.000, n=12. Verified with the full fuzzy + synonym search suites and
-race.Marked draft pending review.