Skip to content

ptx: scan default words without compiling regexes - #15500

Open
limadog9 wants to merge 1 commit into
uutils:mainfrom
limadog9:fix/ptx-default-word-scanner-3142
Open

limadog9 wants to merge 1 commit into
uutils:mainfrom
limadog9:fix/ptx-default-word-scanner-3142

Conversation

@limadog9

Copy link
Copy Markdown
Contributor

Scan default words and break-file separators directly, preserving Unicode word boundaries. This also fixes empty break files dropping all output.

Closes #3142.

Comment thread Cargo.toml Outdated
rand_chacha = { version = "0.10.0" }
rayon = "1.10"
regex = "1.10.4"
regex-syntax = { version = "0.8", features = ["unicode-perl"] }

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we really need a new direct dependency just for is_word_character?

@sylvestre

Copy link
Copy Markdown
Contributor

The PR is about performance, so could you please share before/after numbers from src/uu/ptx/benches/ptx_bench.rs?
this with hyperfine too

Comment thread src/uu/ptx/src/ptx.rs Outdated
Self::Default => regex_syntax::is_word_character(c),
Self::Traditional => !matches!(c, ' ' | '\t' | '\n'),
Self::Break(chars) => !chars.contains(&c),
Self::Regex(_) => unreachable!(),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please avoid the unreachable!(), maybe keep the -W regex out of this enum?

Comment thread src/uu/ptx/src/ptx.rs Outdated
};
let mut chars = line.char_indices().peekable();
std::iter::from_fn(move || {
if let Self::Regex(_) = self {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

checking the variant on every call is a bit odd, could you please return a boxed iterator (or an Either) built once instead?

Comment thread src/uu/ptx/src/ptx.rs Outdated
"[^ \t\n]+".to_owned()
}
}
let skip_nonletters = arg_reg.as_deref() == Some("\\w+")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hard to read, could it be matches!(matcher, WordMatcher::Default) plus the \w+ case?

@limadog9
limadog9 force-pushed the fix/ptx-default-word-scanner-3142 branch from 0a3707e to 5da38a4 Compare October 18, 2026 22:50
@codspeed

codspeed Bot commented Oct 19, 2026 •

Copy link
Copy Markdown

Merging this PR will improve performance by 12.26%

⚡ 5 improved benchmarks
✅ 5 untouched benchmarks
🆕 2 new benchmarks
⏩ 462 skipped benchmarks1

Performance Changes

Mode Benchmark BASE HEAD Efficiency
⚡ Simulation ptx_input_references_long_lines[1000] 1.5 s 1.3 s +14.69%
⚡ Simulation ptx_input_references_short_lines[100000] 12.5 s 11 s +13.76%
⚡ Simulation ptx_long_lines[100] 1.2 s 1.1 s +11.71%
⚡ Simulation ptx_short_lines[100000] 11 s 9.9 s +10.82%
⚡ Simulation ptx_tex[10000] 1,041.4 ms 943.6 ms +10.37%
🆕 Memory ptx_non_ascii[10000] N/A 4.6 MB N/A
🆕 Simulation ptx_non_ascii[10000] N/A 190.9 ms N/A

Tip

Curious why performance improved? Comment @codspeedbot explain why performance improved on this PR, or directly use the CodSpeed MCP with your agent.


Comparing limadog9:fix/ptx-default-word-scanner-3142 (1f194f5) with main (6f60c70)2

Open in CodSpeed

Footnotes

  1. 462 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. ↩

  2. No successful run was found on main (48f2a33) during the generation of this report, so 6f60c70 was used instead as the comparison base. There might be some changes unrelated to this pull request in this report. ↩

Comment thread src/uu/ptx/src/ptx.rs
}
}

fn context_word(config: &Config, line: &str) -> (usize, usize) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this duplicates the gnu_ext -> Default/Traditional choice from WordFilter::new, could it be shared?

Comment thread src/uu/ptx/src/ptx.rs
} else {
static WORD: std::sync::LazyLock<Regex> =
std::sync::LazyLock::new(|| Regex::new(r"^\w$").unwrap());
WORD.is_match(c.encode_utf8(&mut [0; 4]))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this still runs a regex for every non-ascii char, could you please share numbers on a non-ascii input too?

Comment thread src/uu/ptx/src/ptx.rs Outdated
word_regex: String,
matcher: WordMatcher,
word_regex: Option<Regex>,
explicit_regex: bool,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

isn't explicit_regex just word_regex.is_some()? an invalid -W should rather error out than give empty output

@limadog9
limadog9 force-pushed the fix/ptx-default-word-scanner-3142 branch from 5da38a4 to 7dcc4a6 Compare October 19, 2026 19:35
Scan ASCII words directly and retain cached Unicode word classification.
Share the GNU/traditional default choice, compile explicit -W patterns
once, and report invalid patterns instead of producing empty output.
Add a Unicode benchmark and invalid-pattern regression coverage.

Validation: 54 integration tests pass, with Clippy and benchmark checks.
On 10,000-line samples, release medians over 12 runs were:
ASCII: 33.5 ms at the base, 29.1 ms here, 15.5 ms GNU.
Unicode: 100.7 ms at the base, 97.8 ms here, 25.3 ms GNU.
Measurements used one CPU, C.UTF-8 and no PGO; outputs match the base.
Unicode results remain close to the base rather than showing a large gain.

Refs uutils#3142.
@limadog9
limadog9 force-pushed the fix/ptx-default-word-scanner-3142 branch from 7dcc4a6 to 1f194f5 Compare October 19, 2026 19:52

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ptx: Do not use regex when -W options is not provided

2 participants