perf(builtin): guaranteed-linear KMP fallback for substring search - #4119
Conversation
Codex CLI review —
|
There was a problem hiding this comment.
Pull request overview
This PR strengthens StringView::find/rev_find by replacing the dense-candidate fallback path behind the SIMD two-anchor search with a KMP-based fallback, guaranteeing worst-case O(n + m) time on adversarial inputs while keeping the existing fast paths intact.
Changes:
- Replaces the two-anchor fallback scanners with forward/reverse KMP fallbacks to guarantee linear-time behavior.
- Updates public API comments to document the new worst-case linearity guarantee.
- Adds targeted blackbox tests and dedicated adversarial benchmarks to validate correctness and measure worst-case behavior.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| builtin/string_methods.mbt | Updates find/rev_find doc comments to reflect worst-case linear behavior via fallback. |
| builtin/string_find_code_unit.mbt | Implements KMP failure-table + forward/reverse KMP fallbacks and wires them into the two-anchor search cutover. |
| builtin/string_find_fallback_test.mbt | Adds end-to-end tests that force the cutover and validate correctness vs naive references (both directions). |
| builtin/string_find_adversary_bench_test.mbt | Adds benchmarks for adversarial worst-case inputs plus fast-path guard cases. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| // expensive than direct comparison. Cut over to the guaranteed-linear KMP | ||
| // fallback after enough false candidates (the 65th failed verification | ||
| // trips `failures > 64`); the cap keeps pre-cutover verification work at | ||
| // O(pattern), so find/rev_find stay O(target + pattern) even on | ||
| // adversarial inputs. |
There was a problem hiding this comment.
Fixed — the comment now names both triggers: the early ratio check (failures > 4 + scanned / 8, tripping as soon as the 5th failure for candidates packed near the scan start) and the hard cap (failures > 64), and notes the failure count at cutover is at most 65 either way, which is what preserves the O(pattern) pre-cutover bound. Codex CLI (ultra) verified both claims (ratio-first at failure 5 requires scanned < 8; cutover always by failure 65).
| // SIMD two-anchor substring search. The cutover triggers on the 65th | ||
| // failed candidate verification (`failures > 64`), so every test here | ||
| // manufactures more than 65 positions where both anchor code units match |
There was a problem hiding this comment.
Fixed — the header now explains both triggers and states explicitly that these tests force the hard-cap path: the decoys space their false candidates one period (82 units) apart, so failures > 4 + scanned/8 is unsatisfiable (scanned = 82·(f−1)) and the cutover deterministically lands on failures > 64. The early ratio path now gets randomized coverage from the new QuickCheck property over dense two-symbol texts (string_find_quickcheck_test.mbt).
Coverage Report for CI Build 6240Coverage increased (+0.01%) to 90.747%Details
Uncovered ChangesNo uncovered changes found. Coverage RegressionsNo coverage regressions found. Coverage Stats
💛 - Coveralls |
The SIMD two-anchor find/rev_find already cut over to a fallback after
enough dense false-anchor candidates, but that fallback was a naive
scalar scan — still O(target * pattern) on adversarial inputs where
both anchors and long pattern prefixes recur throughout the target
(e.g. needle 'a'*32 + 'Z' + 'a'*31 over all-'a' text). This replaces
the naive fallback with KMP, making find/rev_find O(target + pattern)
in the worst case: the existing cutover budget trips on the 65th
failed verification (`failures > 64`), capping pre-fallback
verification at O(pattern) work, and the fallback itself
is linear. The failure-table allocation is paid only after cutover,
i.e. exclusively on pathological inputs. Resolves the standing TODO on
find/rev_find ("consider using Two-Way algorithm to ensure linear time
complexity") — KMP behind the existing budget gives the same guarantee
with less machinery.
The reverse fallback runs forward KMP over the prefix that can still
contain a hit and keeps the rightmost match, continuing through
overlaps via the failure table.
The idea comes from PR #3786 (mizchi), whose dense-case analysis and
verified-fallback design predate the SIMD two-anchor work that
superseded its scanner; this lands its remaining piece.
Benchmarks (moon bench --release, main -> this branch):
adversary find m=64 n=4096: native 48.6 -> 6.1us (8.0x),
js 453 -> 27.3us (16.6x), wasm-gc 112 -> 10.6us (10.6x)
adversary rev_find m=64 n=4096: native 48.8 -> 7.0us,
js 456 -> 27.4us, wasm-gc 112 -> 12.4us
scaling adversary m=512 n=65536: native 7.20ms -> 91us (79x),
js 56.2ms -> 433us (130x), wasm-gc 15.0ms -> 175us (86x) —
the O(n*m) -> O(n+m) signature
fast paths (dense miss, rare hit): par on all backends; the SIMD
two-anchor path is untouched.
Tests: whitebox KMP tests over periodic/overlapping patterns and
boundary starts; blackbox end-to-end tests built from decoy blocks that
each contribute exactly one false-anchor candidate, so both directions
provably exhaust the 65-failure budget before the interesting region —
dense-anchor misses, hits beyond the cutover point (including a first
occurrence that straddles the decoy/hits boundary), reverse cutover
resolving overlapping hits to the rightmost start, and forward/reverse
naive-reference cross-checks over fast-path and fallback
configurations. string_find_adversary_bench_test.mbt commits the
adversarial workloads so the numbers above are reproducible.
Mutation-verified: zeroing the failure table fails 2 tests; dropping
the overlap continuation in the reverse fallback fails 2.
Codex CLI review (ultra, round 1): production implementation verified
sound by exhaustive binary-alphabet modeling (267M helper-contract
cases, 89M full-route cases, no counterexample vs naive search),
including the handoff off-by-ones and the skipped-position reasoning;
three P2 findings all in tests/prose — a whitebox test argument that
violated the reverse helper's bound (out-of-bounds unsafe_get, fixed
to 7 and the precondition documented), an overlap test that never
reached the fallback (rebuilt with decoy blocks), and overstated
naive-reference coverage (rebuilt with both directions and both
paths).
Copilot review addressed: the cutover comments now describe BOTH
triggers — the early ratio check (failures > 4 + scanned/8, tripping
as soon as the 5th failure for candidates packed near the scan start)
and the hard cap (failures > 64) — and the fallback-test header notes
its spaced decoys deterministically force the hard-cap path.
QuickCheck added (string_find_quickcheck_test.mbt): a general
random-text/needle equivalence property against the naive references
(dense two-symbol texts randomly exercise the early ratio cutover),
and a fallback-forcing property that surrounds a randomized hit/decoy
mixture with 70 corrupted blocks per side (anchors intact, one
interior char flipped), fuzzing the KMP fallback itself with
randomized needle shapes. Zeroing the failure table is falsified by
the property after 2 cases.
Round 2 (ultra) verified the fixes: "All three P2 fixes are correct
and complete. Whitebox bound is safely 7, with the precondition
documented. Decoy arithmetic, forward result 5699, reverse result 41,
and 65th-failure cutovers check out. Naive references use correct
inclusive bounds and cover fast/fallback paths in both directions."
Round 3 (ultra) verified the Copilot-prompted prose and the QuickCheck
layer: "Ratio cutover can first occur on failure 5 when scanned < 8;
cutover is always by failure 65. Period-82 decoys never trigger the
ratio branch and deterministically hit the hard cap. All 2,835 valid
corrupted-block constructions preserve anchors, differ from the needle,
and cannot form accidental cross-block matches. Both directions force
fallback before the mixture."
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Codex CLI <codex@openai.com>
Signed-off-by: Codex CLI <codex@openai.com>
9ff985d to
80aa7cf
Compare
Round 3 — Copilot comments addressed + QuickCheck layer, Codex CLI
|
Summary
Makes
StringView::find/rev_findworst-case O(n + m). The SIMD two-anchor search already cuts over to a fallback after dense false-anchor candidates (the 65th failed verification), but that fallback was a naive scalar scan — still O(n·m) on adversarial inputs where both anchors and long pattern prefixes recur throughout the target. This replaces both fallbacks with KMP: the cutover budget caps pre-fallback verification at O(m), and the fallback is linear, so the whole search is linear. The failure-table allocation is paid only after cutover — exclusively on pathological inputs; the SIMD fast paths are untouched.Resolves the standing TODO on
find/rev_find("consider using Two-Way algorithm to ensure linear time complexity") — KMP behind the existing budget gives the same guarantee with less machinery. The idea originates in #3786 (@mizchi), whose dense-case analysis predates the SIMD two-anchor work that superseded its scanner; this lands its remaining piece (analysis).The reverse fallback runs forward KMP over the prefix that can still contain a hit and keeps the rightmost match, continuing through overlaps via the failure table.
Benchmarks (
moon bench --release, main → this PR; fixtures committed instring_find_adversary_bench_test.mbt)findm=64 n=4096rev_findm=64 n=4096The scaling row is the O(n·m) → O(n+m) signature. (Adversary needle:
'a'×k + 'Z' + 'a'×(k−1)over all-'a'text — every position passes both anchor tests and fails deep in verification.)Tests
string_find_fallback_test.mbt): built from decoy blocks that each contribute exactly one false-anchor candidate, so both directions provably exhaust the 65-failure budget — dense-anchor misses; hits beyond the cutover point (including a first occurrence that straddles the decoy/hits boundary at 5699, which the naive cross-check itself exposed); reverse cutover resolving overlapping hits to the rightmost start; and forward/reverse naive-reference sweeps over fast-path and fallback configurations.Review — Codex CLI at
ultra, 2 roundsRound 1 proved the production implementation sound by exhaustive binary-alphabet modeling — 267M helper-contract cases and 89M full-route cases against naive search, no counterexample — including the handoff off-by-ones and the skipped-position reasoning, then requested changes on three P2 findings, all in tests/prose: a whitebox test argument that violated the reverse helper's bound (an out-of-bounds
unsafe_getin the test — fixed, precondition documented), an overlap test that never actually reached the fallback (rebuilt on the decoy construction), and overstated naive-reference coverage (rebuilt, both directions). Round 2 verified all three fixes and signed off.Signed-off-by: Codex CLI codex@openai.com
Validation
moon test: 7463/7463 (builtin+string 3227/3227)moon check --warn-list +unnecessary_annotationclean,moon fmtapplied, no.mbtichange (all new functions private)🤖 Generated with Claude Code