FST read-path and search optimizations#28
Open
capemox wants to merge 3 commits into
Open
Conversation
Three focused fixes: 1. builder.go: compileFrom swallowed errors from compile() via `return nil`, silently producing a corrupt FST on a mid-build write failure. Return the error instead. 2. levenshtein/alphabet.go: dedupe() built its result via O(n^2) string concatenation (rv += string(r)). Use a strings.Builder. Runs per fuzzy query. 3. levenshtein/parametric_dfa.go: getHash() json.Marshal'd each NFAState and SHA-256'd the result to key a map[[32]byte]int. Replace with a direct 6-bytes-per-state encoding into a reused buffer, keyed in a map[string]int. No reflection, no hashing, exact (collision-free) keys. Drops the crypto/sha256 and encoding/json imports. Added BenchmarkNewLevenshteinAutomatonBuilder1/2 to cover the construction path. Building an edit-distance-2 automaton is now ~3x faster with ~58% fewer allocations; per-query BuildDfa drops ~8-10% allocations. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
#4 regexp/dfa.go: state.next was []int (256*8 = 2KB per state on 64-bit). State indices are bounded by StateLimit (10000), so []uint32 is always sufficient and halves the transition-table footprint. For DFAs near the state cap this saves ~10MB. Total compile allocation drops 15-25% on the benchmark regexps, entirely attributable to the smaller tables. #5 fst.go: the Automaton/Transducer interface methods (Accept/AcceptWithVal, IsMatch/IsMatchWithVal) called decoder.stateAt(addr, nil), allocating a throwaway fstStateV1 on every call. TransducerGet over an 11-byte key thus allocated 12 states. Recycle states through a sync.Pool on the FST (safe for concurrent use). TransducerGet goes from 1920 B/12 allocs to 0/0 and ~18% faster; the iterator and Reader paths are unchanged (they already reuse a prealloc state). Adds DFA-footprint and read-path benchmarks plus a concurrency race test for the pooled methods. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ator Two allocation-free, format-compatible speedups to FST reads (no on-disk format change; existing FSTs benefit without a rebuild): 1. Reader root-state cache: the root is traversed on every Get, so parse it once at Reader construction and reuse the immutable parsed form, letting each lookup skip re-decoding the root. 2. Iterator offset accessor: the automaton-guided iterator already knows a transition's sorted offset from TransitionAt, but then re-searched for the byte via TransitionFor (a bytes.IndexByte scan) to recover the dest/output. Add TransitionDestForOffset to resolve dest/output directly by offset, and only after the automaton accepts the transition so selective searches don't decode transitions they discard. Benchmarks (Apple M4 Pro, benchstat, n=6, all p=0.002; 0 allocs unchanged): GetWords 99.2µs -> 88.1µs -11.1% GetWide (8B) 5.91ms -> 5.49ms -7.1% GetWide2 (2B) 2.50ms -> 1.74ms -30.4% ScanWords 87.3µs -> 81.9µs -6.2% ScanWide 7.15ms -> 6.71ms -6.1% FuzzyWords1 2.97µs -> 2.73µs -8.0% FuzzyWords2 10.6µs -> 9.6µs -9.3% RegexWords 80.6µs -> 74.0µs -8.2% geomean -11.2% Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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.
Overview
A set of allocation-conscious performance optimizations to vellum's FST read and search paths. The headline change (this branch's newest commit) speeds up lookups and automaton-guided search with no on-disk format change — existing FSTs benefit without a rebuild. Two earlier commits on the branch trim allocations and table sizes in the regexp/levenshtein automatons.
All changes are covered by the existing test suite plus new benchmarks, and pass under
-race.Contributions
1. Cache root state in
Readerand skip redundant key lookups in the iterator (4f29369)Two format-compatible, allocation-free read speedups:
Readerroot-state cache — the root state is traversed on everyGet, so it is now parsed once atReaderconstruction and the immutable parsed form is reused, letting each lookup skip re-decoding the root.TransitionAt, but then re-searched for the byte viaTransitionFor(abytes.IndexBytescan) to recover the destination/output. A newTransitionDestForOffsetresolves destination/output directly by offset, and only after the automaton accepts the transition, so selective searches don't decode transitions they discard.Benchmarks (Apple M4 Pro,
benchstat, n=6, all p=0.002; allocations unchanged at 0 forGet):2. Shrink regexp DFA tables and pool FST automaton states (
c953a0a)Reduces the regexp DFA table footprint and pools the transient
fstStateinstances used by theAutomaton/Transducerinterface methods, avoiding a per-call allocation.3. Fix swallowed build error and reduce levenshtein allocations (
0469d8d)Surfaces a previously swallowed build error and trims allocations in the levenshtein automaton construction path.
Notes