Conversation
ad2619c to
8385621
Compare
| num-prime = "0.5.0" | ||
| num-traits = "0.2.19" | ||
| fancy-regex = { version = "0.19.1", default-features = false, features = [ | ||
| fancy-regex = { version = "0.19.2", default-features = false, features = [ |
There was a problem hiding this comment.
perhaps we could move the feature selection to src/uucore/Cargo.toml
Merging this PR will not alter performance
Comparing 
Footnotes
|
8385621 to
47d63dc
Compare
This comment was marked as outdated.
This comment was marked as outdated.
47d63dc to
a427a2b
Compare
|
GNU testsuite comparison: |
a427a2b to
257233c
Compare
This comment was marked as resolved.
This comment was marked as resolved.
846bee8 to
20880e0
Compare
wtcpython
changed the title
20880e0 to
cd53de5
Compare
|
@codspeedbot fix this regression |
Fixed in 3d05af4. What the report was actually showingThe regression was real, not noise: every memory benchmark in the tree gained exactly the same amount — +16 allocations, +4,752 bytes allocated, +4,187 bytes peak — including utilities that never compile a regex. Only the cheapest benchmarks crossed the reporting threshold, which is why the list looks arbitrary. I reproduced it locally with the CodSpeed CLI:
Cause
Growing that shared file therefore charges every benchmark. The eight new FixThe ids the error resource defines are now generated at build time and checked before the bundle is built, so a lookup that cannot be in there costs a
The regression is gone, and every benchmark drops the error-resource parse it was never supposed to pay — expect improvements across the board in the next report rather than a flat comparison. Validation: One caveat on the original report: its base was 94811cc rather than your merge base, and several benchmarks were compared across different CPU models. That did not affect this diagnosis — the memory numbers are byte-identical on both base runs — but it is worth reading the "Environment Differences" section before trusting any sub-10% simulation move on this repo. Summary
Was this response helpful? React with 👍 or 👎 to help us improve CodSpeedBot. |
|
@sylvestre The agent successfully pushed wtcpython@3d05af4 to the PR’s source branch, but GitHub did not synchronize the PR with the new commit. I’m investigating 👀 |
6ad71f6 to
73b9fbc
Compare
|
@sylvestre ready |
73b9fbc to
09138e9
Compare
df85213 to
518923c
Compare
518923c to
452610d
Compare
452610d to
03f321a
Compare
03f321a to
a531ab8
Compare
a531ab8 to
208e15b
Compare
208e15b to
1f0ee94
Compare
1f0ee94 to
f9b82c1
Compare
There was a problem hiding this comment.
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Copilot review overview
Review effort: Lite
Findings: 1
Open (3)
String::ends_withdoes not accept a predicate closure here, so this will not compile. To preserve… · Newverify_range_quantifieris called, which is avoidable overhead… · NewClocale,[[:alpha:]]may not… · New
| if curr == ']' && re_string.ends_with(|c| c != '\\' && c != '[' && c != '^') { | ||
| in_bracket = false; | ||
| } |
| let re = Regex::new(r"^([0-9]*,[0-9]*|[0-9]+)$").expect("valid regular expression"); | ||
| if let Ok(Some(captures)) = re.captures(&quantifier) { | ||
| let matched = captures.get(0).map_or("", |m| m.as_str()); | ||
| match matched.split_once(',') { | ||
| Some(("", "")) => Ok(()), | ||
| Some((x, "") | ("", x)) if x.parse::<i16>().is_ok() => Ok(()), | ||
| Some((_, "") | ("", _)) => Err(RegexError::TooBigRangeQuantifierIndex), | ||
| Some((f, l)) => match (f.parse::<i16>(), l.parse::<i16>()) { | ||
| (Ok(f), Ok(l)) if f > l => Err(RegexError::InvalidBracketContent), | ||
| (Ok(_), Ok(_)) => Ok(()), | ||
| _ => Err(RegexError::TooBigRangeQuantifierIndex), | ||
| }, | ||
| None if matched.parse::<i16>().is_ok() => Ok(()), | ||
| None => Err(RegexError::TooBigRangeQuantifierIndex), | ||
| } | ||
| } else { | ||
| Err(RegexError::InvalidBracketContent) | ||
| } |
| #[test] | ||
| #[cfg_attr(wasi_runner, ignore = "WASI: no locale data, every locale is C")] | ||
| fn test_regex_posix_character_classes() { | ||
| new_ucmd!() |
The error-only resource is parsed on the first lookup that misses every
ordinary bundle. That is not only an error path: a binary that cannot
resolve its own strings misses on every id it asks for, and a bench binary
calling `uumain` directly is exactly that. Every benchmark in the tree
therefore parsed the whole resource -- 106k of the 319k instructions
hostname_basic measures, and 16 allocations it never frees.
Growing the resource consequently charged every benchmark, whether or not
the utility can reach the new strings. The eight regex messages added here
moved hostname_basic from 318,823 to 335,034 instructions (+5.1%) and every
utility's peak memory by 4.2 KB, in utilities that never compile a regex.
Generate the ids the resource defines at build time and check that before
building the bundle, so a lookup that cannot be in there costs a match
instead of a parse:
hostname_basic instructions
main (94811cc) 318823
this branch 335034
this branch, this commit 212632
Measured with
`codspeed run --mode simulation -- cargo codspeed run -p uu_hostname`.
The expr benchmarks are unchanged (1,448,685 against 1,448,761 on main),
and expr still reports its regex diagnostics, in English and in French.
f9b82c1 to
17f28aa
Compare
No description provided.