Skip to content

fix: isMatchDFA concurrent safety — prefilter rejection only (#137)#145

Merged
kolkov merged 1 commit into
mainfrom
hotfix/concurrent-ismatch-safety
Mar 19, 2026
Merged

fix: isMatchDFA concurrent safety — prefilter rejection only (#137)#145
kolkov merged 1 commit into
mainfrom
hotfix/concurrent-ismatch-safety

Conversation

@kolkov

@kolkov kolkov commented Mar 19, 2026

Copy link
Copy Markdown
Contributor

Summary

isMatchDFA prefilter candidate loop called e.dfa.SearchAtAnchored concurrently from RunParallel. Shared lazy DFA is NOT thread-safe for concurrent lazy state construction. On ARM64 without SIMD prefilters, every candidate hit DFA → cache corruption → 1.7GB allocs, 1s+ per op on M2 Max.

Fix: prefilter used for fast rejection only (one Find call). Verification falls through to DFA.IsMatch (read-mostly when cache warm).

Test: TestConcurrentCaseInsensitivePrefilter — 8 goroutines × 100 iterations, match + no-match paths.

Reported by @tjbrains on Apple M2 Max (#137).

Test plan

  • Full test suite passes
  • Concurrent test passes (8 goroutines)
  • Lint clean
  • CI passes

isMatchDFA prefilter candidate loop called e.dfa.SearchAtAnchored
concurrently from RunParallel — shared lazy DFA NOT thread-safe.
On ARM64 without SIMD: cache corruption, 1.7GB allocs, 1s+ per op.

Fix: prefilter for fast rejection only, DFA.IsMatch for verification.
Added TestConcurrentCaseInsensitivePrefilter (8 goroutines x 100 iters).
Reported by @tjbrains on M2 Max.
@kolkov kolkov force-pushed the hotfix/concurrent-ismatch-safety branch from 71312bd to a92ec46 Compare March 19, 2026 06:27
@github-actions

Copy link
Copy Markdown

Benchmark Comparison

Comparing main → PR #145

Summary: geomean 118.4n 118.3n -0.04%

⚠️ Potential regressions detected:

Accelerate/memchr3-4       339.0n ± ∞ ¹   339.8n ± ∞ ¹  +0.24% (p=0.024 n=5)
geomean                               ³                +0.00%               ³
geomean                               ³                +0.00%               ³
geomean                         ³                +0.00%               ³
geomean                         ³                +0.00%               ³
AhoCorasickManyPatterns/coregex_25_patterns-4           72.05n ± ∞ ¹     73.54n ± ∞ ¹     +2.07% (p=0.016 n=5)
ASCIIOptimization/WithASCII-4                           9.428n ± ∞ ¹     9.677n ± ∞ ¹     +2.64% (p=0.024 n=5)
ASCIIOptimization_Issue79/medium_WithoutASCII-4         630.9n ± ∞ ¹     643.8n ± ∞ ¹     +2.04% (p=0.032 n=5)
LangArenaLogParser/suspicious-4                         1.188m ± ∞ ¹     1.202m ± ∞ ¹     +1.16% (p=0.032 n=5)
LangArenaLogParser/ips-4                                96.18µ ± ∞ ¹     97.33µ ± ∞ ¹     +1.20% (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

@codecov

codecov Bot commented Mar 19, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 80.00000% with 2 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
meta/ismatch.go 80.00% 1 Missing and 1 partial ⚠️

📢 Thoughts on this report? Let us know!

@kolkov kolkov merged commit ee6b351 into main Mar 19, 2026
8 of 9 checks passed
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