perf(strscan): make the compile cache thread-safe (sync.Map) - #1
Merged
Conversation
The compiled-regexp cache that lets a repeated pattern source compile only once (instead of recompiling on every Scan/Match/Skip/Check and *Until) was backed by a plain, unsynchronized map. Caching by source string is correctness-safe — flags are embedded inline in the source the caller passes (e.g. "(?imx)..."), so the compiled regexp for a given source is deterministic — but the bare map raced under concurrent Scanners: `go test -race` reliably reports a data race when many goroutines scan at once, and a concurrent map write can crash the process. Replace the map with a process-wide sync.Map, consulted at the single onigmo.Compile site: - Load on the read-mostly hot path is lock-free. - LoadOrStore collapses concurrent compile-misses of the same source to one shared *Regexp (losers drop their duplicate), so every Scanner reuses the same compiled value. - Compile errors are still surfaced as "no match" and are NOT cached, keeping error behavior byte-for-byte identical to compiling every call. Results are unchanged (caching is purely internal). The shared *Regexp's Match is itself concurrency-safe, so Scanners can share one compiled pattern. Add strscan_bench_test.go (a lexer-shaped scan loop, cached vs recompile-every- call) and tests for concurrent scanning (the -race regression guard) and the malformed-not-cached path. Benchmark (Apple M4 Max, go1.26.4, CGO_ENABLED=0), full tokenizing pass: ScanLoopCached ~0.61 ms/op 1,095,383 B/op 28,120 allocs/op ScanLoopNoCache ~28.0 ms/op 46,284,000 B/op 669,328 allocs/op i.e. the compile cache is ~45x faster and ~24x fewer allocations than recompiling every Scan, with zero behavior change. 100% coverage, gofmt/go vet/-race clean, CGO=0, builds on all 6 arches; the MRI-4.0.5 differential oracle still passes. Co-Authored-By: Claude Opus 4.8 <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.
What
The compiled-regexp cache that lets a repeated pattern source compile only once — instead of recompiling via
onigmo.Compileon everyScan/Match/Skip/Check(and*Until) — was backed by a plain, unsynchronizedmap[string]*onigmo.Regexp. Caching by the source string is correctness-safe (the flags a caller wants are embedded inline in the source, e.g.(?imx)..., so the compiled regexp for a given source is deterministic), but the bare map raced under concurrent Scanners:go test -racereliably reports a data race when many goroutines scan at once, and a concurrent map write can crash the process.The fix
Replace the map with a process-wide
sync.Map, consulted at the singleonigmo.Compilesite:Loadon the read-mostly hot path is lock-free.LoadOrStorecollapses concurrent compile-misses of the same source to one shared*Regexp(losers drop their duplicate), so every Scanner reuses the same compiled value.Results are unchanged — caching is purely internal. The shared
*Regexp'sMatchis itself concurrency-safe, so Scanners can share one compiled pattern.Tests added
strscan_bench_test.go— a lexer-shaped scan loop, cached vs recompile-every-call, to quantify the win.TestCompileCacheConcurrent— many Scanners, many goroutines: the permanent-raceregression guard (fails on the old map, passes onsync.Map).TestCompileCacheMalformedNotCached— a malformed pattern keeps missing identically, and a later valid pattern still compiles.Benchmark (Apple M4 Max, go1.26.4,
CGO_ENABLED=0)Full tokenizing pass over a representative input with a small fixed pattern set:
ScanLoopCached(this PR)ScanLoopNoCache(recompile every Scan)The compile cache is ~45x faster and uses ~24x fewer allocations than recompiling on every
Scan, with zero behavior change.Conformance
compile100%, total 100%).gofmt/go vet/go test -raceclean;CGO_ENABLED=0.ruby 4.0.5.🤖 Generated with Claude Code