seg: 2-3x faster merge compression via Aho-Corasick matcher and cover-DP optimizations - #21625
Conversation
|
no idea what this all means; but results are good. Will check why in coming days |
Byte-level automaton producing the same maximal-match set as MatchFinder3+deduplicateMatches in a single O(len) scan per word, without per-word suffix-array construction. Caches per-position automaton states so a word sharing a prefix with its predecessor (sorted streams) resumes at the first differing byte. FuzzLongestMatch gains a brute-force oracle for the new matcher: the patricia tree loses an existing key when a proper prefix of it is inserted later (see committed corpus entry), so MF1/MF2/MF3 under-report matches on prefix-nested dictionaries and cannot serve as the oracle.
All changes produce byte-identical compressed output: - single-match fast path (DP provably always includes the match) - DP deque as reversed flat slice with indexed truncation, replacing Ring - upper-bound skip: compression+matchLen-4 below the running max cannot win even on score tie-break - monotone-bound early exit: cell compression is non-increasing along the scan, so the first bound failure ends it; the truncation point is found by binary search - virtual initial window cells: the initial cells are identical, so they are an interval contributing one candidate - position-code maps replaced by bounded arrays in both passes
b184da3 to
0f7609c
Compare
|
Updated: the original |
|
I advise separate PR:
|
0f7609c to
5a44ee8
Compare
|
Done — split per your advice:
|
|
Mainnet validation (storage merge 8960→9085, 1.61 GiB in, 35.3M keys, inputs copied out of a live datadir, content verified in all runs):
Two notes vs the bloatnet results:
|
There was a problem hiding this comment.
Pull request overview
This PR focuses on significantly speeding up storage-domain snapshot merges (single-threaded, compression-bound) while preserving byte-identical compressed output. It replaces the previous per-word suffix-array/patricia-based matching with an Aho–Corasick automaton and applies multiple DP/encoding hot-path optimizations, plus moves btindex existence-filter hashing off the critical path.
Changes:
- Add a new
patriciaAho–Corasick automaton + per-goroutine matcher with prefix-resume caching for sorted-key streams. - Optimize
coverWordByPatterns(DP) and position-frequency accounting to reduce allocations and hot-path map lookups. - Parallelize
.kveiexistence-filter hashing by scanning via a clonedseg.Readerinbtindex.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| db/seg/seg_auto_rw.go | Adds Reader.CloneReader() to create an independent reader over the same decompressor for concurrent scans. |
| db/seg/patricia/testdata/fuzz/FuzzLongestMatch/34174f1279b62c1e | Adds fuzz corpus entry covering prefix-nested patterns (regression reproducer). |
| db/seg/patricia/patricia_fuzz_test.go | Extends fuzzing to validate AC matcher vs a brute-force oracle and checks warm/fresh matcher consistency. |
| db/seg/patricia/aho_corasick.go | Introduces the Aho–Corasick automaton + ACMatcher implementation with fail links and longest-match propagation. |
| db/seg/parallel_compress.go | Switches compressor matching to AC, adds DP fast-path and deque/pos-count optimizations, and reduces map pressure in position-code lookup. |
| db/datastruct/btindex/btree_index.go | Moves existence-filter hashing to a separate goroutine using a cloned reader to avoid serializing BT key-walk behind hashing. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
5a44ee8 to
61f1070
Compare
AskAlexSharov
left a comment
There was a problem hiding this comment.
Let’s merge it after release/3.5 branch creation
## Problem `Compressor.AddWord` decided when a superstring window was full from `len(c.superstring)`. But skipped (non-sampled) windows never append to that buffer, so once `SamplingFactor>1` reached the first skipped window the buffer stopped growing, the overflow branch never fired again, the window counter froze, and **the pattern dictionary was built from the first `superstringLimit` (16MB) of the file only.** This affects every `SamplingFactor>1` user: `seg.DefaultCfg` and `BlockCompressCfg` both use `SamplingFactor=4`, so tx/bodies/headers segments have been built from first-16MB dictionaries. ## Fix Track scanned bytes per window in `scannedBytes`, advanced on **every** word regardless of sampling, and use it (not `len(c.superstring)`) for the overflow check. Window boundaries now advance through skipped windows, so `SamplingFactor` honestly samples every Nth window across the whole file. While here, the windowing logic moved into a small `advanceScan` helper so the byte accounting can't drift from the rollover it feeds, and the send condition is now `len(superstring) > 0` (non-empty ⟺ sampled): this stops pushing empty buffers to the workers and only fetches a fresh pool buffer when one is actually handed off. ## Scope / impact - **Affected:** `seg.DefaultCfg`, `BlockCompressCfg` (`SamplingFactor=4`) — tx/bodies/headers segments now build from a true 25% whole-file sample (slower build, smaller output; offset by erigontech#21625's matcher speedups). - **Not affected:** `DomainCompressCfg` / `HistoryCompressCfg` (`SamplingFactor=1`) — for `SamplingFactor=1` the new accounting is provably identical to the old, so domain/history snapshots stay **byte-identical** (verified by the unchanged checksum tests). Addresses the core bug in erigontech#21628. Per-config `SamplingFactor` re-tuning (the issue's other open thread) is intentionally left out of this PR. ## Testing - New `TestCompressSamplingCoversWholeFile` pins the invariant "the number of windows a file splits into must not depend on `SamplingFactor`" — fails on the old code (SF=1 → 90 windows, SF=4 → stuck at 1), passes after the fix. - Full `db/seg` suite passes under `-race`; existing checksum-asserting tests unchanged (single-window output byte-identical).
…-Corasick matcher and cover-DP optimizations (#21625) (#21946) Cherry-pick of #21625 to `performance`. Replaces the per-word pattern matcher (Aho-Corasick) and optimizes the cover DP in `db/seg` merge compression. Output byte-identical (or marginally smaller). Clean cherry-pick, no `performance`-specific adaptations. | workload | main | this PR | |---|---|---| | bloatnet 16-step merge (766M in, 13.8M keys) | 382.6s | **135.0s (2.8x)** | | mainnet 6-file merge (1.61G in, 35.3M keys) | 1204.2s | **612.9s (2.0x)** | Cover phase (matcher + DP) 241.2s → 14.6s (16.5x); SAIS dictionary extraction untouched.
…ntech#21998) Since erigontech#21625, the compressor's cover phase uses only the Aho-Corasick matcher. The old suffix-array `PatriciaTree` / `MatchFinder1/2/3` path has no remaining callers — it's dead code (the sole repo-wide importer, `db/seg/parallel_compress.go`, uses AC exclusively). ## Change - delete `patricia_tree.go`, `patricia_flat.go`, and `patricia_flat_test.go` - relocate the `Match` / `Matches` types (the only part AC still needs) into `aho_corasick.go` - trim `patricia_fuzz_test.go` to the AC + brute-force-oracle path; drop `FuzzPatricia` (it fuzzed the removed `node`) Net −1069 LOC. `db/seg` and `db/seg/patricia` build and tests pass. ## Closes erigontech#21626 The prefix-loss bug (`Insert` drops an existing key when a proper prefix is inserted later) lived in the now-removed `PatriciaTree.Insert`. The AC matcher is unaffected — it's validated against a brute-force oracle in `FuzzLongestMatch`, which this PR keeps.

Merging storage-domain snapshot files is single-threaded and compression-bound. This PR replaces the per-word pattern matcher and optimizes the cover DP; output is byte-identical (or marginally smaller, see notes).
Benchmarked on real storage files through
DomainRoTx.mergeFiles(incl..bt/.kveibuild),Workers=1as at chain-tip, merged content verified against an independent re-merge:Where the speedup comes from
CPU profile of a representative single-worker bloatnet 16-step merge, attributing the cover phase by sub-component:
FindLongestMatches).bt+ I/O + gcThe matcher carries ~80% of the win, the cover-DP rewrite ~19%. End-to-end speedup is Amdahl-capped near 3x: SAIS dictionary extraction is untouched and is now ~79% of remaining merge time — parallelizing it (size-neutral) is the next lever (see Notes).
The matcher's responsibility is - given the 64K limit candidate pattern dictionary, run it through each word and find which pattern matches the word. The cover DP finds optimal subset of these matches which provides best compression.
Changes
seg/patricia: Aho–Corasick matcher replaces per-word suffix-array matching (SAIS+LCP+bit-level patricia walk); same maximal-match set in one O(len) scan, resuming from the shared prefix with the previous word (merge keys are sorted)seg:coverWordByPatternsDP — single-match fast path; flat-slice deque replacingRing; upper-bound skip; monotone-bound early exit with binary-search truncation; virtual initial window cells; position-code maps → arraysVerification
FuzzLongestMatchextended with a brute-force oracle for the AC matcher (3.5M+ execs clean)make lintclean;db/seg,db/seg/patricia,db/statetests passNotes
mmapwithmadv_sequential#21482's sequential-view readahead on cold files