Skip to content

fix: NFA candidate loop — partialCoverage instead of IsComplete#150

Closed
kolkov wants to merge 1 commit intomainfrom
hotfix/nfa-candidate-loop-guard
Closed

fix: NFA candidate loop — partialCoverage instead of IsComplete#150
kolkov wants to merge 1 commit intomainfrom
hotfix/nfa-candidate-loop-guard

Conversation

@kolkov
Copy link
Copy Markdown
Contributor

@kolkov kolkov commented Mar 23, 2026

Summary

Fix NFA candidate loop guard — IsComplete() blocked prefilter for ALL incomplete prefilters, causing 22x regression on Kostya's errors pattern.

  • Root cause: IsComplete() guard blocked NFA candidate loop for prefix-only prefilters where all alternation branches ARE represented. Only overflow partial-coverage prefilters are unsafe.
  • Rust approach: PikeVM integrates prefilter as skip-ahead inside the engine (pikevm.rs:1293-1299), not as external correctness gate. Partial coverage safe because NFA continues scanning.
  • Fix: Added partialCoverage flag on literal.Seq (set only on overflow truncation). NFA candidate loop uses !partialCoverage instead of IsComplete().

Results (Kostya's LangArena benchmark, 7.2MB × 10 iterations)

Pattern v0.12.14 v0.12.17 (regressed) After fix Rust
errors 90ms 1984ms 109ms 37ms
Total 1801ms 3761ms ~2000ms 312ms

Test plan

  • go test ./... — all pass
  • TestStdlibCompatibility — 38/38 PASS, 0 SKIP
  • golangci-lint run — 0 issues
  • CI: 3 platforms
  • regex-bench + macOS ARM64

…lete

IsComplete() guard blocked prefilter candidate loop for ALL incomplete
prefilters, including prefix-only ones where all alternation branches are
represented. This caused 22x regression on Kostya's errors pattern
(1984ms vs 90ms on v0.12.14).

Root cause: Rust integrates prefilter as skip-ahead INSIDE PikeVM
(pikevm.rs:1293-1299), not as external correctness gate. When NFA states
are empty, prefilter skips ahead. Partial coverage is safe because NFA
continues scanning if prefilter misses.

Fix: Added partialCoverage flag on literal.Seq (set only on overflow
truncation). NFA candidate loop uses !partialCoverage guard instead of
IsComplete(). DFA paths retain IsComplete() where needed.

errors: 1984ms -> 109ms. Stdlib compat: 38/38 PASS.
@codecov
Copy link
Copy Markdown

codecov bot commented Mar 23, 2026

Codecov Report

❌ Patch coverage is 41.66667% with 7 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
literal/seq.go 0.00% 4 Missing ⚠️
literal/extractor.go 0.00% 1 Missing and 1 partial ⚠️
meta/find_indices.go 80.00% 0 Missing and 1 partial ⚠️

📢 Thoughts on this report? Let us know!

@github-actions
Copy link
Copy Markdown

Benchmark Comparison

Comparing main → PR #150

Summary: geomean 120.6n 120.0n -0.54%

⚠️ Potential regressions detected:

geomean                               ³                +0.00%               ³
geomean                               ³                +0.00%               ³
geomean                         ³                +0.00%               ³
geomean                         ³                +0.00%               ³
AhoCorasickManyPatterns/coregex_10_patterns-4           58.41n ± ∞ ¹    59.95n ± ∞ ¹     +2.64% (p=0.032 n=5)
AhoCorasickManyPatterns/coregex_25_patterns-4           72.71n ± ∞ ¹    74.10n ± ∞ ¹     +1.91% (p=0.016 n=5)
AhoCorasickManyPatterns/coregex_50_patterns-4           77.79n ± ∞ ¹    80.59n ± ∞ ¹     +3.60% (p=0.008 n=5)
MatchAnchoredLiteral/medium_match-4                     8.123n ± ∞ ¹    8.734n ± ∞ ¹     +7.52% (p=0.008 n=5)
MatchAnchoredLiteral/long_match-4                       8.208n ± ∞ ¹    8.745n ± ∞ ¹     +6.54% (p=0.008 n=5)
AnchoredLiteralVsStdlib/stdlib_short-4                  254.0n ± ∞ ¹    257.0n ± ∞ ¹     +1.18% (p=0.008 n=5)

Full results available in workflow artifacts. CI runners have ~10-20% variance.
For accurate benchmarks, run locally: ./scripts/bench.sh --compare

kolkov added a commit that referenced this pull request Mar 23, 2026
Integrate prefilter inside PikeVM search loop as skip-ahead (pikevm.rs:1293).
When NFA has no active threads, PikeVM jumps to next candidate via
prefilter.Find() instead of byte-by-byte scan.

Safe for partial-coverage prefilters — NFA processes all branches from
each candidate position. This is architecturally cleaner than external
candidate loop guards (partialCoverage flag still used for external BT
candidate loop as BoundedBacktracker has no integrated skip-ahead).

Also includes PR #150 changes: partialCoverage flag on literal.Seq,
NFA candidate loop guard uses partialCoverage instead of IsComplete().

errors pattern: 1984ms -> 120ms. la_suspicious: 38/38 stdlib PASS.
kolkov added a commit that referenced this pull request Mar 24, 2026
* fix: NFA candidate loop guard — use partialCoverage instead of IsComplete

IsComplete() guard blocked prefilter candidate loop for ALL incomplete
prefilters, including prefix-only ones where all alternation branches are
represented. This caused 22x regression on Kostya's errors pattern
(1984ms vs 90ms on v0.12.14).

Root cause: Rust integrates prefilter as skip-ahead INSIDE PikeVM
(pikevm.rs:1293-1299), not as external correctness gate. When NFA states
are empty, prefilter skips ahead. Partial coverage is safe because NFA
continues scanning if prefilter misses.

Fix: Added partialCoverage flag on literal.Seq (set only on overflow
truncation). NFA candidate loop uses !partialCoverage guard instead of
IsComplete(). DFA paths retain IsComplete() where needed.

errors: 1984ms -> 109ms. Stdlib compat: 38/38 PASS.

* perf: PikeVM integrated prefilter skip-ahead (Rust approach)

Integrate prefilter inside PikeVM search loop as skip-ahead (pikevm.rs:1293).
When NFA has no active threads, PikeVM jumps to next candidate via
prefilter.Find() instead of byte-by-byte scan.

Safe for partial-coverage prefilters — NFA processes all branches from
each candidate position. This is architecturally cleaner than external
candidate loop guards (partialCoverage flag still used for external BT
candidate loop as BoundedBacktracker has no integrated skip-ahead).

Also includes PR #150 changes: partialCoverage flag on literal.Seq,
NFA candidate loop guard uses partialCoverage instead of IsComplete().

errors pattern: 1984ms -> 120ms. la_suspicious: 38/38 stdlib PASS.

* perf: flat DFA transition table — eliminate pointer chase in hot loop

Replace double indirection (stateList[id].transitions[class]) with flat
transition table (flatTrans[sid*stride + class]) in searchFirstAt hot loop.
Also replace State.IsMatch() with compact matchFlags[sid] bool slice.

Fast path now works with state ID only — no *State pointer needed.
State struct accessed only in slow path (determinize, word boundary).

Inspired by Rust regex-automata hybrid/dfa.rs Cache.trans flat layout.

Kostya benchmark: 3.60s -> 2.56s (1.4x faster).
bots pattern restored to v0.12.14 baseline (278ms vs 287ms).
Stdlib compat: 38/38 PASS.

* perf: 4x loop unrolling in searchFirstAt (Rust approach)

Unroll DFA hot loop 4x — process 4 bytes per iteration when all
transitions are in flat table (no unknown/dead states). Falls to
single-byte slow path on any special state.

Marginal improvement on x86 with SIMD prefilters (branch predictor
handles single-byte well). May help more on ARM64 where branch
prediction is less aggressive.

Reference: Rust hybrid/search.rs:195-221.
Stdlib compat: 38/38 PASS.

* perf: apply flat DFA transition table to ALL search functions

Extend flat table optimization from searchFirstAt to all 6 DFA search
functions: searchAt, searchEarliestMatch, searchEarliestMatchAnchored,
SearchReverse, SearchReverseLimited, IsMatchReverse.

Hot loop pattern: ft[int(sid)*stride + classIdx] replaces
stateList[id].transitions[class] — eliminates pointer chase.
State struct accessed only in slow path (determinize, word boundary).

Kostya benchmark: 2.56s -> 2.28s (+12%).
errors pattern: 109ms -> 81ms (better than v0.12.14 baseline 90ms).
Stdlib compat: 38/38 PASS.

* fix: restore DFA prefilter skip-ahead for incomplete prefilters

IsComplete() guard in findIndicesDFA/findIndicesDFAAt blocked prefilter
skip-ahead for incomplete prefilters (memmem, Teddy with prefix-only
literals). But DFA verifies full pattern at candidate — skip is always safe.

This was the root cause of sessions (229ms -> 36ms), api_calls (245ms ->
95ms), post_requests (259ms -> 114ms) regressions.

Kostya benchmark total: 2.28s -> 1.62s (FASTER than v0.12.14 baseline 1.80s!).
Stdlib compat: 38/38 PASS.

* perf: DFA prefilter skip-ahead at start state (Rust approach)

When DFA returns to start state with no match in progress, use prefilter
to skip ahead to next candidate instead of byte-by-byte scanning.
Applied to searchFirstAt and searchAt (bidirectional DFA path).

This is the Rust approach (hybrid/search.rs:232-258): prefilter is called
inside the DFA loop when a start state is detected, not externally.

peak_hours: 197ms -> 90ms (2.2x faster, gap vs Rust: 9x -> 4x).
Kostya total: 1.62s -> 1.38s (15% faster).
Stdlib compat: 38/38 PASS.

* docs: update CHANGELOG for v0.12.18

* perf: flat DFA transition table in SearchAtAnchored

Apply flat table to SearchAtAnchored — called for every prefilter
candidate verification in bidirectional DFA path. Eliminates pointer
chase in the most frequent DFA hot path.

Kostya benchmark: 1.38s -> 1.17s (15% faster).
Total improvement vs v0.12.14: 1.80s -> 1.17s (35% faster).
Stdlib compat: 38/38 PASS.

* perf: flat DFA transition table in isMatchWithPrefilter and findWithPrefilterAt

Apply flat table to last 2 remaining functions with old Transition() calls.
No more State pointer chase in ANY DFA hot loop.

Kostya benchmark: 1.17s -> 1.19s (stable, tokens 116ms->51ms).
All DFA search functions now use flatTrans[sid*stride+class].
Stdlib compat: 38/38 PASS.

* docs: update ROADMAP and CHANGELOG for v0.12.18

* fix: guard getState/IsMatchState against 386 int overflow

On 386, int(StateID(0xFFFFFFFF)) = -1 (int is 32-bit).
getState and IsMatchState used int(id) for slice indexing,
causing panic: index out of range [-1].

Fix: check sid >= DeadState before int cast.
DeadState (0xFFFFFFFE) and InvalidState (0xFFFFFFFF) are
sentinel values not present in stateList/matchFlags.

* fix: use safeOffset for all flat table indexing — 386 int overflow

On 386, int is 32-bit. int(StateID(0xFFFFFFFE)) = -2, causing
negative slice index panic in flat table lookups.

Added safeOffset() helper using uint arithmetic (always positive).
Replaced all 23 occurrences of int(sid)*stride in hot loops.
safeOffset inlines — zero overhead on 64-bit.

* fix: safeOffset guard for DeadState/InvalidState on 386

uint multiply overflows on 386: uint(0xFFFFFFFE)*uint(20) wraps around.
Guard with sid >= DeadState check — returns MaxInt so bounds check fails
safely. Normal state IDs (small values) take fast path without branch.

* docs: update README benchmark table and ROADMAP for v0.12.18
@kolkov
Copy link
Copy Markdown
Contributor Author

kolkov commented Mar 24, 2026

Superseded by #151 — all changes included in v0.12.18 (PikeVM skip-ahead + partialCoverage guard).

@kolkov kolkov closed this Mar 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant