Repository navigation
Conversation
| rand_chacha = { version = "0.10.0" } | ||
| rayon = "1.10" | ||
| regex = "1.10.4" | ||
| regex-syntax = { version = "0.8", features = ["unicode-perl"] } |
There was a problem hiding this comment.
do we really need a new direct dependency just for is_word_character?
|
The PR is about performance, so could you please share before/after numbers from src/uu/ptx/benches/ptx_bench.rs? |
| Self::Default => regex_syntax::is_word_character(c), | ||
| Self::Traditional => !matches!(c, ' ' | '\t' | '\n'), | ||
| Self::Break(chars) => !chars.contains(&c), | ||
| Self::Regex(_) => unreachable!(), |
There was a problem hiding this comment.
please avoid the unreachable!(), maybe keep the -W regex out of this enum?
| }; | ||
| let mut chars = line.char_indices().peekable(); | ||
| std::iter::from_fn(move || { | ||
| if let Self::Regex(_) = self { |
There was a problem hiding this comment.
checking the variant on every call is a bit odd, could you please return a boxed iterator (or an Either) built once instead?
| "[^ \t\n]+".to_owned() | ||
| } | ||
| } | ||
| let skip_nonletters = arg_reg.as_deref() == Some("\\w+") |
There was a problem hiding this comment.
hard to read, could it be matches!(matcher, WordMatcher::Default) plus the \w+ case?
0a3707e to
5da38a4
Compare
Merging this PR will improve performance by 12.26%
Performance Changes
Tip Curious why performance improved? Comment Comparing Footnotes
|
| } | ||
| } | ||
|
|
||
| fn context_word(config: &Config, line: &str) -> (usize, usize) { |
There was a problem hiding this comment.
this duplicates the gnu_ext -> Default/Traditional choice from WordFilter::new, could it be shared?
| } 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])) |
There was a problem hiding this comment.
this still runs a regex for every non-ascii char, could you please share numbers on a non-ascii input too?
| word_regex: String, | ||
| matcher: WordMatcher, | ||
| word_regex: Option<Regex>, | ||
| explicit_regex: bool, |
There was a problem hiding this comment.
isn't explicit_regex just word_regex.is_some()? an invalid -W should rather error out than give empty output
5da38a4 to
7dcc4a6
Compare
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.
7dcc4a6 to
1f194f5
Compare
Scan default words and break-file separators directly, preserving Unicode word boundaries. This also fixes empty break files dropping all output.
Closes #3142.