perf: compile user regex patterns once per planned expression

[dwsmith1983]

Which issue does this PR close?

No dedicated issue. Related to #4942, whose description says the remaining Regex::new calls were already hoisted into statics; these three user-pattern call sites were still compiling per batch on current main.

Rationale for this change

regexp_extract, regexp_extract_all, and split called Regex::new on the user's pattern inside the per-batch evaluation path, so every 8192-row batch paid a full regex compile. rlike in the same crate already compiles once at plan time; these three could not take that exact shape because they are scalar functions created by name, and the pattern only arrives per invocation as a scalar argument.

What changes are included in this PR?

Each planned expression now owns a one-slot PatternCache (new string_funcs/pattern_cache.rs): compile on first use, reuse while the pattern string is unchanged, recompile if it ever differs (split's serde does not require a literal pattern, so the cache tolerates changes rather than assuming a constant). Regex clones share the compiled program, so handing out clones per batch is an Arc bump. Error messages are byte-identical and an invalid pattern still fails at the same phase as before.

Numbers on an M-series mac: criterion regexp_extract goes from 862us to 705us per 8192-row batch (about 18% faster), and a small-batch run (512 rows, 5000 batches) is 2.1x faster since compile cost is amortized over fewer rows. The split bench is flat because its case uses a literal delimiter, which takes the non-regex fast path. One known unknown worth stating: the cache uses a Mutex and the benches are single-threaded, so contention under DataFusion's intra-task parallelism is unmeasured. The fast path is a lock, a string compare, and a clone, so it should be negligible, and the lock also prevents duplicate compiles on a cold cache.

How are these changes tested?

Seven new tests: three cache unit tests (compile-once, recompile-on-change, invalid pattern does not poison the slot), three multi-batch tests pinning one compile across batches per function via a test-only counter, and one pinning that an invalid split pattern still errors at evaluation. Full crate suites pass (670 spark-expr, 212 core), clippy with warnings denied and fmt are clean, and the Scala side was exercised through CometStringExpressionSuite (33 tests, includes the native split path) and CometRegExpJvmSuite (46 tests).

[sunchao]

Reviewed b4d9152367c8a0233beaa8b9817a69c0969e0c11 against 8729f6e6adf7091e18a48670e790d4ba8fd41e51. No verified P1/P2 findings.

A focused check of the unchanged cache source passed six tests, including concurrent cold access, different-pattern replacement and invalid-pattern recovery. This was a cache-only check, not the full Comet/Spark suite. Current-head workflows report action_required. The earlier HEAD's green checks are not current-head validation.

Could you add a matched BASE/HEAD multithreaded benchmark with shared-UDF and per-worker controls, using 1/2/4/8 workers and 512/8192-row batches? Please cover cold and warm caches, alternating patterns, and a regex delimiter for split, and report throughput, batch latency and allocations while checking equal results and confirming the native path. Regex::clone() shares the compiled program but creates a fresh search-cache pool, so this would measure both contention and the per-batch clone cost.

[dwsmith1983]

@sunchao
Ran the benchmark on a 10 core Apple M5 (4P + 6E), comparing this branch against the base commit it sits on. The harness builds the UDFs through create_comet_physical_fun and calls invoke_with_args with the pattern arriving as a scalar argument on every invoke, which is the native path and the reason the cache exists. Matrix: regexp_extract, regexp_extract_all, and split with a regex delimiter ([,;|]+), shared and per worker UDF instances, 1/2/4/8 workers, 512 and 8192 row batches, warm and alternating pattern regimes, 4M rows per cell, two full replicates. Outputs were verified byte identical between main and this PR in every cell.

Warm regime, per worker instances (matches real plans, where the pattern is a literal and each task gets its own expression instance):

function	workers	rows/batch	main Mrows/s	this PR Mrows/s	change
regexp_extract	1	512	6.9	17.9	+159%
regexp_extract	8	512	5.8	62.8	+986%
regexp_extract	8	8192	79.3	111.9	+41%
regexp_extract_all	8	512	3.2	6.5	+103%
split	8	512	25.6	30.2	+18%
split	8	8192	31.7	31.8	0%

Main anti-scales on small batches: 8 threads run slower than 1 because every thread recompiles the pattern per batch and the compiles hammer the allocator. This PR scales near linearly. Allocations per 512 row batch for regexp_extract drop from 988 to 82 (the compile alone is roughly 900 allocations and 0.7 MB). Per batch latency follows the same shape, for example 699us mean / 1271us p99 down to 62us / 106us in the 8 worker 512 row cell.

Worst case for the one slot cache, a pattern that alternates on every single invoke: within 2 percent of main across all three functions and both batch sizes, since the miss path pays the same compile main always pays plus an uncontended mutex. Cold first invoke on a fresh instance is also unchanged (for example 415us on main vs 403us here for regexp_extract on 8192 rows).

One honest caveat: an artificial control where a single UDF instance is shared across 8 threads simultaneously regresses regexp_extract_all on 8192 row batches by 6 to 29 percent. The threads contend on the shared compiled Regex's internal scratch pool in that setup, while main sidesteps it by compiling privately per batch, which is the same behavior causing the anti-scaling above. That configuration does not occur in Comet since each task deserializes its own plan and gets its own expression instance, and regexp_extract and split win in shared mode anyway.

[sunchao]

@dwsmith1983 Thanks for covering the requested matrix. Could you attach the runnable harness/commands, exact baseline and PR commit SHAs, dependency/build settings, and per-cell results for both replicates, including the regressing shared-instance cases?

Per-task plan ownership does not rule out sharing within a task. Source inspection shows that Comet passes sort-key expressions directly to SortExec. In DataFusion 54.1.0, ExternalSorter::in_mem_sort_stream uses spawn_buffered for multiple retained batches once the reservation reaches sort_in_place_threshold_bytes. The cloned orderings retain the same expression/UDF, which can then be evaluated concurrently on Comet's multithread runtime. Could you add a native sort case with regexp_extract_all directly in the sort key, no LIMIT, 8192-row batches, and enough unsorted input to reach that branch? A one-/eight-worker comparison, with the native plan and evidence of overlapping calls to the same UDF, would test whether the adverse control matters here. A precomputed regex column would not exercise that sharing. This is source evidence for the path, not a reproduced end-to-end slowdown.

Could you also revisit the scratch-pool attribution? PatternCache::get_or_compile returns an owned Regex clone, and the pinned regex-automata 0.4.16 Regex::clone creates a fresh scratch-cache pool. Sharing the compiled program is not sharing that scratch pool. The reported slowdown may still be real, but its cause needs the harness or profiling evidence. I have not independently rerun these timings.

[dwsmith1983]

Thanks for pushing on all three points. You were right on both technical claims, so taking them in order.

Harness and raw data: https://gist.github.com/dwsmith1983/e46e22c1c594b4f5120515c773f2b3ef has the full harness source, exact build and run commands, both commit SHAs, toolchain and dependency versions, and per-cell CSVs for both replicates including the regressing shared-instance cells.

Sort path: your reading of ExternalSorter checks out and the repro confirms it. With the exact plan shape Comet produces (SortExec, no fetch, single partition, 128 x 8192-row batches so the reservation is well past sort_in_place_threshold_bytes), a tracking shim around one UDF instance measured max 9 concurrent in-flight evaluations at 8 runtime workers, and even 2 at 1 worker since the merge evaluates concurrently with a spawned sort task. On that path regexp_extract_all as the sort key was 1.45x slower than base at 1 worker and 2.2x at 8. regexp_extract as the key was parity to slightly faster.

Attribution: you were right that my scratch-pool explanation was wrong. Clone creates a fresh private pool (meta/regex.rs 1916-1926), so scratch state is never shared. The real mechanism, isolated in a micro benchmark in the gist, is per-row refcount traffic: captures_iter creates a Captures per row via create_captures, which is Captures::all(self.group_info().clone()), an Arc clone against the program-owned GroupInfo, plus one more Captures clone per match. With every thread holding clones of one compiled program, that single refcount cache line bounces across cores and caps throughput regardless of thread count. A variant with one clone per thread, no lock and no per-invoke clone still collapses identically, which rules out the mutex and the clone itself. regexp_extract is immune because it reuses one CaptureLocations across rows, and split never creates a Captures.

That pointed at the fix, now pushed: regexp_extract_all drives iteration with find_iter (identical span semantics, verified against the crate's shared iterator code and pinned with empty-match and multibyte edge tests) and resolves groups through captures_read_at into one CaptureLocations reused per batch, same as regexp_extract. Rerun results: the sort scenario goes from 2.2x slower to 8 percent faster than base at 8 workers, shared-instance and per-instance modes are now identical, and removing the per-match allocations lets the function scale near linearly to 8 workers (7.1 to 72 Mrows/s at 8 workers, where base and the previous head were both stuck near 7). Outputs stay byte identical across base and both head builds in every cell. Fix verification tables and CSVs are in the gist as well.

[sunchao]

Reviewed the capture-location update in 84f9ee006fc94fb592b61439d9d520d484af616d and the pinned benchmark follow-up. No new P1/P2 findings.

A focused regex-only comparison passed for 86 patterns and 1,512 strings, including empty matches, optional groups, anchors, word boundaries, and UTF-8 offsets. The benchmarked fix has the same relevant source as this HEAD, and the supplied sort results show recovery of the reported regression. Those timings are author-run evidence, not my measurements. I did not run the full Comet/Spark suite; the final CI snapshot had 37 passing checks, 28 queued/running, and 6 skipped.