grep: replace onig with fancy-regex by wtcpython · Pull Request #117 · uutils/grep

wtcpython · 2026-09-11T06:35:04Z

Closes #60
Closes #20

Copilot

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

oech3 · 2026-09-11T09:41:31Z

Add Closes #60 and Closes #20 to 1st comment.

github-actions · 2026-09-11T09:56:33Z

GNU grep testsuite comparison:

Test results comparison:
  Current:   TOTAL: 128 / PASSED: 82 / FAILED: 25 / SKIPPED: 21
  Reference: TOTAL: 128 / PASSED: 78 / FAILED: 29 / SKIPPED: 21

Changes from main branch:
  TOTAL: +0
  PASSED: +4
  FAILED: -4

New test failures (3):
  - glibc-infloop
  - pcre-invalid-utf8-input
  - unibyte-binary

Test improvements (7):
  + backslash-s-and-repetition-operators
  + dfa-invalid-utf8
  + hangul-syllable
  + multibyte-white-space
  + pcre
  + utf8-bracket
  + z-anchor-newline

codspeed-hq · 2026-09-11T09:56:55Z

Merging this PR will regress 1 benchmark

⚡ 6 improved benchmarks
❌ 1 regressed benchmark
✅ 3 untouched benchmarks
⏩ 17 skipped benchmarks¹

Warning

Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

	Benchmark	`BASE`	`HEAD`	Efficiency
❌	`regex_no_match`	51.5 ms	59.1 ms	-12.83%
⚡	`extended_icase`	271.5 ms	57.4 ms	×4.7
⚡	`invert_match`	157.9 ms	42.3 ms	×3.7
⚡	`only_matching`	107.4 ms	72.7 ms	+47.81%
⚡	`filename_lineno_color`	107.6 ms	72.9 ms	+47.7%
⚡	`recursive_no_binary`	50 ms	42 ms	+19.08%
⚡	`context`	56.8 ms	48.8 ms	+16.42%

Tip

Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.

_{Comparing wtcpython:replace-onig-with-fancy-regex (d499dbf) with main (b69057f)}

17 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩

Copilot

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

oech3 · 2026-09-11T10:12:05Z

-        if options.contains(RegexOptions::REGEX_OPTION_IGNORECASE) {
-            case_fold_flag &= !onig_sys::INTERNAL_ONIGENC_CASE_FOLD_MULTI_CHAR;
+/// Convert POSIX Basic Regular Expression (BRE) to standard regex syntax for fancy-regex.
+fn transpile_bre(pattern: &str) -> UResult<String> {


Did you implement it at uutils/expr and copy-pasted to grep and findutils? If so, we should move it to uucore.

lhecker

I have seen this code before as it is almost line for line identical to the code that Claude spat out when this was being prototyped earlier this year. Passing the standard of the current test suites does not mean it's good code.

A great example for that is the O(n) single codepoint parser, where n = line length. Did the test suite catch that -w expressions are now O(n^2)? No. Neither did it catch the leftmost longest rule being broken.

I'll be closing this PR because I'm fairly confident that you cannot fix the leftmost longest issue in a reasonable way. See rust-lang/regex#1126.

lhecker · 2026-09-11T11:33:53Z

+            let next_char = match std::str::from_utf8(&line[end..]) {
+                Ok(s) => s.chars().next(),
+                Err(e) if e.valid_up_to() > 0 => {
+                    std::str::from_utf8(&line[end..end + e.valid_up_to()])
+                        .ok()
+                        .and_then(|s| s.chars().next())
+                }
+                Err(_) => None,
+            };


An unbounded O(n) algo to get the next codepoint? That can be done better, I'm sure.

lhecker · 2026-09-11T11:39:36Z

    }

    /// Word-boundary check `-w`.
-    /// NOTE that `-w` does not check both sides, unlike `\b` in a regex.


You should reinstate this comment. It's still true.

lhecker · 2026-09-11T11:43:01Z

+/// Sort alternation branches by descending length so that leftmost-first engines
+/// match the longest alternative first (POSIX leftmost-longest semantics).


How would you even know what the longest branch is? In aaaaa|a* the 2nd branch is shorter and yet longer.

Proof:

printf 'aaaaaa' | cargo run -- -E -o 'aaaaa|a*'

This issue is as far as I'm aware unfixable and so this PR can be closed. See rust-lang/regex#1126.

this is fixable in fancy-regex because it is a backtracking engine, I prepared a PR to enable it: fancy-regex/fancy-regex#281

lhecker · 2026-09-11T11:55:21Z

-        if options.contains(RegexOptions::REGEX_OPTION_IGNORECASE) {
-            case_fold_flag &= !onig_sys::INTERNAL_ONIGENC_CASE_FOLD_MULTI_CHAR;
+/// Convert POSIX Basic Regular Expression (BRE) to standard regex syntax for fancy-regex.
+fn transpile_bre(pattern: &str) -> UResult<String> {


This and the one below are a very hard read.

wtcpython · 2026-09-11T12:14:18Z

I’m really sorry about the issues here, and thanks for taking the time to review this and point them out. I’ll close this PR.

lhecker · 2026-09-11T12:17:05Z

You don't have to be sorry! I do appreciate when people take their time to contribute improvements. My primary feedback is to give your code a closer read before submitting it as AIs are smart but very overconfident.

lhecker · 2026-09-13T01:06:31Z

@wtcpython You can reopen this PR and put it into "draft" if you'd like. Once the fancy-regex PR has been merged and released we can revisit this PR.

lhecker · 2026-09-13T12:35:23Z

I did some benchmarks with the just released fancy-regex 0.19.2 and my grep -iE '\b(error|warning|failed)\b' query that I sometimes use. (Personally, it's the only one I use grep on large files for.)

I found that this PR unfortunately regresses performance by ~8x in my tests. It's worth noting that onig too is already ~10x slower than GNU grep. However, fancy-regex would make that two orders of magnitude, which (at least in my use cases) makes it perceptible.

I toyed around with it for a bit and found that that about half of that is due to BytesMode::UnicodeBytes. I suspect that this can be fixed. The regex crate is roughly on par with GNU grep. Does fancy-regex not defer to the regex crate for "simple" patterns? I had assumed so.

keith-hall · 2026-09-13T12:42:34Z

Does fancy-regex not defer to the regex crate for "simple" patterns? I had assumed so.

it does indeed, although in this case \b isn't delegated to regex-automata because regex-automata struggles with unicode word boundaries in some sub-engines/strategies. Probably in ascii mode (as opposed to unicode) we should treat it as delegatable but I don't think it has been done yet.
Also, make sure you enable .seek(true) on the regex builder for best performance.

EDIT: I forgot to mention, with leftmost-longest mode, a lot more work has to be done in the fancy-regex VM, it can't simply delegate to regex-automata. Seek mode should improve things a lot though.

oech3 · 2026-09-13T13:29:47Z

perf(-dfa-full) feature is disabled both of coreutils and grep as it just increased size of binary. Is this worth to enable it?

wtcpython · 2026-09-13T13:56:48Z

I tried enabling seek(true) on the fancy-regex builder.

Benchmarked on a 641 MB / 10,000,000-line log:

Version	Mean time
main (Oniguruma)	2.283 s
PR without `seek`	22.911 s
PR with `seek`	698.4 ms
GNU grep	343.2 ms

lhecker · 2026-09-13T15:33:47Z

That is really nice! But you may have to repeat the GNU grep benchmark. 3ms would mean it ran at 213GB/s which is unrealistic.

Copilot AI lite review requested due to automatic review settings September 11, 2026 06:35

Copilot AI reviewed Sep 11, 2026

oech3 mentioned this pull request Sep 11, 2026

grep: allow wasm32-wasip1 cargo check #53

Open

grep: replace onig with fancy-regex

d729ce2

wtcpython force-pushed the replace-onig-with-fancy-regex branch from d499dbf to d729ce2 Compare September 11, 2026 10:05

Copilot AI review requested due to automatic review settings September 11, 2026 10:05

Copilot AI reviewed Sep 11, 2026

oech3 reviewed Sep 11, 2026

View reviewed changes

lhecker requested changes Sep 11, 2026

View reviewed changes

wtcpython closed this Sep 11, 2026

wtcpython deleted the replace-onig-with-fancy-regex branch September 11, 2026 12:14

lhecker mentioned this pull request Sep 11, 2026

find, locate: replace onig with fancy-regex uutils/findutils#861

Closed

keith-hall mentioned this pull request Sep 12, 2026

Add leftmost longest match semantics fancy-regex/fancy-regex#280

Closed

		/// Sort alternation branches by descending length so that leftmost-first engines
		/// match the longest alternative first (POSIX leftmost-longest semantics).

Uh oh!

Conversation

wtcpython commented Sep 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

oech3 commented Sep 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Sep 11, 2026

Uh oh!

codspeed-hq Bot commented Sep 11, 2026

Merging this PR will regress 1 benchmark

Performance Changes

Footnotes

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

oech3 Sep 11, 2026

Choose a reason for hiding this comment

Uh oh!

lhecker left a comment

Choose a reason for hiding this comment

Uh oh!

lhecker Sep 11, 2026

Choose a reason for hiding this comment

Uh oh!

lhecker Sep 11, 2026

Choose a reason for hiding this comment

Uh oh!

lhecker Sep 11, 2026

Choose a reason for hiding this comment

Uh oh!

keith-hall Sep 12, 2026

Choose a reason for hiding this comment

Uh oh!

lhecker Sep 11, 2026

Choose a reason for hiding this comment

Uh oh!

wtcpython commented Sep 11, 2026

Uh oh!

lhecker commented Sep 11, 2026

Uh oh!

lhecker commented Sep 13, 2026

Uh oh!

lhecker commented Sep 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

keith-hall commented Sep 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

oech3 commented Sep 13, 2026

Uh oh!

wtcpython commented Sep 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lhecker commented Sep 13, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

wtcpython commented Sep 11, 2026 •

edited

Loading

oech3 commented Sep 11, 2026 •

edited

Loading

lhecker commented Sep 13, 2026 •

edited

Loading

keith-hall commented Sep 13, 2026 •

edited

Loading

wtcpython commented Sep 13, 2026 •

edited

Loading