-
Notifications
You must be signed in to change notification settings - Fork 12.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Eliminate bunch of copies of error codepath from Utf8LossyChunksIter #91244
Conversation
Using a macro to stamp out 7 identical copies of the nontrivial slicing logic to exit this loop didn't seem like a necessary use of a macro. The early return case can be handled by `break` without practically any changes to the logic inside the loop. All this code is from early 2014 (7.5 years old, pre-1.0) so it's possible there were compiler limitations that forced the macro way at the time. Confirmed that `x.py bench library/alloc --stage 0 --test-args from_utf8_lossy` is unaffected on my machine.
(rust-highfive has picked a reviewer for you, use r? to override) |
library/core/src/str/lossy.rs
Outdated
}; | ||
self.source = &[]; | ||
Some(r) | ||
// SAFETY: `i <= self.source.len()` because it only ever increments by 1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i
is incremented up to 3 times in some loop iterations?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's incremented up to 4 times in some loop iterations, but what I meant is: in between every pair of i += 1
, we always compare i
vs self.source.len()
. As soon as an i += 1
puts i
out of bounds, then if safe_get(self.source, i) & 192 != TAG_CONT_U8 { break; }
will terminate the loop because 0 & 192 != 128.
@bors r+ rollup=never |
📌 Commit c6810a5 has been approved by |
☀️ Test successful - checks-actions |
Finished benchmarking commit (94bec90): comparison url. Summary: This change led to very large relevant mixed results 🤷 in compiler performance.
If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf. Next Steps: If you can justify the regressions found in this perf run, please indicate this with @rustbot label: +perf-regression |
I suspect regressions are due to rust-lang/rustc-perf#1105, with query verification hard enabled I cannot reproduce them. So going to remove that tag. |
Using a macro to stamp out 7 identical copies of the nontrivial slicing logic to exit this loop didn't seem like a necessary use of a macro. The early return case can be handled by
break
without practically any changes to the logic inside the loop.All this code is from early 2014 (#12062—nearly 8 years ago; pre-1.0) so it's possible there were compiler limitations that forced the macro way at the time.
Confirmed that
x.py bench library/alloc --stage 0 --test-args from_utf8_lossy
is unaffected on my machine.