Add direct root transition table to aho automaton - #43
Merged
Conversation
Root (state 0) has ~2,500 edges and receives 60-70% of all Next probes, accounting for ~90% of the log-weighted binary-search cost. Derive a token-indexed table for root's children at build/load time and check it in Next when the state is or falls back to root. MatchUnmatchedRepetitiveInput 68.3 -> 82.8 MB/s. Matching benchmarks unchanged since successful matches extend at deep single-edge states and rarely reach root.
There was a problem hiding this comment.
🟡 Changes recommended
BuildRootTable can attempt pathological allocations (or panic) when token IDs are large/sparse, and should defensively bound/disable the table in those cases since Next already has a fallback.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR optimizes the Aho-Corasick automaton’s hot-path transition from the root state by adding a token-indexed root transition table, reducing the cost of repeated root probes during largely non-matching scans.
Changes:
- Add
rootNext []uint32toaho.Automatonand aBuildRootTablehelper to precompute root transitions. - Update
Automaton.Nextto userootNextfor state 0 (or when falling back to root), with binary-search fallback ifrootNextis absent. - Populate
rootNextboth when building an automaton in-memory and when reading one from the corpus, and add tests to ensure correctness.
File summaries
| File | Description |
|---|---|
| internal/corpus/format.go | Calls BuildRootTable after reading an automaton from disk so root transitions are available without format changes. |
| internal/aho/aho.go | Introduces rootNext, builds the root transition table, and updates Next to use it for root-state transitions. |
| internal/aho/aho_test.go | Adds tests to confirm rootNext behavior matches the binary-search path and updates equality checks. |
Review details
- Files reviewed: 3/3 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+305
to
+319
| func (a *Automaton) BuildRootTable() { | ||
| if len(a.EdgeStarts) < 2 { | ||
| return | ||
| } | ||
| end := a.EdgeStarts[1] | ||
| if end == 0 { | ||
| a.rootNext = nil | ||
| return | ||
| } | ||
| table := make([]uint32, a.EdgeTokens[end-1]+1) | ||
| for edge := a.EdgeStarts[0]; edge < end; edge++ { | ||
| table[a.EdgeTokens[edge]] = edge + 1 | ||
| } | ||
| a.rootNext = table | ||
| } |
andrew
added a commit
that referenced
this pull request
Sep 5, 2026
licenses timings re-measured on main after #42 and #43: cargo 0.91 -> 0.85 s, self 0.77 -> 0.74 s, RSS within noise. scancode figures unchanged. The -n default changed to N-1 in scancode-toolkit 32.4.0 (aboutcode-org/scancode-toolkit#4104), not 32.5.0.
andrew
added a commit
that referenced
this pull request
Sep 5, 2026
* Add ScanCode comparison to README and benchmarking docs Numbers measured against scancode-toolkit 32.5.0 on rust-lang/cargo at a07c49a on an 8-core M1 Pro. Reproduction steps and the aggregate-RSS sampling method are in docs/benchmarking.md. * Refresh figures for 0da4c8b and correct -n default attribution licenses timings re-measured on main after #42 and #43: cargo 0.91 -> 0.85 s, self 0.77 -> 0.74 s, RSS within noise. scancode figures unchanged. The -n default changed to N-1 in scancode-toolkit 32.4.0 (aboutcode-org/scancode-toolkit#4104), not 32.5.0. * Note exit 2 behaviour in ScanCode comparison instructions Repositories with unparseable manifests, including deliberately-invalid test fixtures, produce per-file errors and a non-zero exit without affecting the timing or RSS being measured.
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.
Automaton.Nextlooks up transitions per state viaslices.BinarySearchover that state's sorted edge tokens. InstrumentingNextacross a full-history scan of rubygems (241M probes) and gitea (648M probes) showed state 0 receives 63-71% of all probes and, with ~2,500 edges, accounts for 89-93% of the log-weighted search cost. Every token that fails to extend the current match falls back through failure links to root, so non-matching input (most source code) hits root once per token.Adds
rootNext []uint32toAutomaton, a token-indexed table of root's children sized to root's highest edge token (~120 KB with the current corpus).BuildRootTablederives it fromEdgeStartsandEdgeTokens;Buildandcorpus.readAutomatoncall it after assembly, so no corpus format change.Nextconsults the table when the state is or falls back to root, with the existing binary search as a fallback for automata built without it.GOMAXPROCS=1,-benchtime 2s:MatchUnmatchedRepetitiveInput/5000MatchUnmatchedRepetitiveInput/10000MatchUnmatchedRepetitiveInput/20000MatchApacheHash,MatchApacheExact,MatchCorpusHash, andMatchRepeatedShortNoticeare unchanged: successful matches advance through deep single-edge states (already fast-pathed indirectImplicit) and rarely reach root.On a full-history blob scan sharing one
Matcheracross 8 goroutines: rubygems/rubygems 4.58 s -> 3.99 s (-13%), gitea 13.0 s -> 11.5 s (-11%), go-git 447 ms -> 394 ms (-12%). RSS unchanged.Independent of #42; both apply to
Matcher.matchbut touch different packages.