dwsmith1983 commented on PR #5612: URL: https://github.com/apache/datafusion-comet/pull/5612#issuecomment-5536242508
Thanks, that double search was real and it reproduced here at +24 percent and +43 percent on your two cases. Fixed in ea8c0e369: the loop now drives `captures_read_at` alone with one reused `CaptureLocations`, taking the overall match from group 0, so each match is searched once and there is still no per-match allocation. The empty-match rule mirrors `Searcher::try_advance` in regex-automata 0.4.16 (an empty match ending where the previous one ended advances one byte and searches again), and an equivalence test compares the helper against `captures_iter` across 12 patterns including `a*`, `\\b`, and `(?:)`, 12 haystacks including CJK and emoji, and groups 0, 1, 2, and an out-of-range index. Your two cases are now in `benches/regexp_extract_all.rs`. Criterion medians on this machine, base is main 55ae4f20: | case | base | before fix | after fix | after vs base | |---|---:|---:|---:|---:| | 8,192 rows of 123-456-789-123, (\\d+), group 1 | 1.93 ms | 2.39 ms | 1.44 ms | -26% | | 512 rows of 8 KB matched by (a+), group 1 | 59.2 ms | 84.5 ms | 48.2 ms | -19% | | 524,288 rows, no nulls (existing case) | 193.5 ms | 250.2 ms | 163.9 ms | -15% | Full table is in the commit's bench file output; every case is below base after the fix. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
