-
Notifications
You must be signed in to change notification settings - Fork 12.8k
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
optimize EscapeAscii's Display and CStr's Debug #113142
Conversation
the8472
commented
Jun 29, 2023
•
edited
Loading
edited
old: ascii::bench_ascii_escape_display_mixed 17.97µs/iter +/- 204.00ns ascii::bench_ascii_escape_display_no_escape 545.00ns/iter +/- 6.00ns new: ascii::bench_ascii_escape_display_mixed 4.99µs/iter +/- 56.00ns ascii::bench_ascii_escape_display_no_escape 91.00ns/iter +/- 1.00ns
r? @m-ou-se (rustbot has picked a reviewer for you, use r? to override) |
@bors r+ It might be nice to check the code size impact, but my guess is that this isn't really a huge amount of new code. Not sure how much it matters anyway in terms of whether this gets used... |
…iaskrgr Rollup of 10 pull requests Successful merges: - rust-lang#103730 (Added NonZeroXxx::from_mut(_unchecked)?) - rust-lang#113142 (optimize EscapeAscii's Display and CStr's Debug) - rust-lang#118799 (Stabilize single-field offset_of) - rust-lang#119613 (Expose Obligations created during type inference.) - rust-lang#119752 (Avoid ICEs in trait names without `dyn`) - rust-lang#120132 (Teach tidy about line/col information for malformed features) - rust-lang#120135 (SMIR: Make the remaining "private" fields actually private) - rust-lang#120148 (`single_use_lifetimes`: Don't suggest deleting lifetimes with bounds) - rust-lang#120150 (Stabilize `round_ties_even`) - rust-lang#120155 (Don't use `ReErased` to detect type test promotion failed) r? `@ghost` `@rustbot` modify labels: rollup
|
||
pub(crate) fn into_inner(self) -> Option<I> { |
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.
Inline or it inlines itself good enough?
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.
Well, it worked in the microbenchmarks at least. With mir inlining, the cross-crate auto-inlining which was recently added and std being compiled with 1CGU that's probably enough.
If perf shows regressions I'll reconsider.
|
||
pub(crate) fn into_inner(self) -> I { |
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.
here too.
…iaskrgr Rollup of 10 pull requests Successful merges: - rust-lang#103730 (Added NonZeroXxx::from_mut(_unchecked)?) - rust-lang#113142 (optimize EscapeAscii's Display and CStr's Debug) - rust-lang#118799 (Stabilize single-field offset_of) - rust-lang#119613 (Expose Obligations created during type inference.) - rust-lang#119752 (Avoid ICEs in trait names without `dyn`) - rust-lang#120132 (Teach tidy about line/col information for malformed features) - rust-lang#120135 (SMIR: Make the remaining "private" fields actually private) - rust-lang#120148 (`single_use_lifetimes`: Don't suggest deleting lifetimes with bounds) - rust-lang#120150 (Stabilize `round_ties_even`) - rust-lang#120155 (Don't use `ReErased` to detect type test promotion failed) r? `@ghost` `@rustbot` modify labels: rollup
Rollup merge of rust-lang#113142 - the8472:opt-cstr-display, r=Mark-Simulacrum optimize EscapeAscii's Display and CStr's Debug ``` old: ascii::bench_ascii_escape_display_mixed 17.97µs/iter +/- 204.00ns ascii::bench_ascii_escape_display_no_escape 545.00ns/iter +/- 6.00ns new: ascii::bench_ascii_escape_display_mixed 4.99µs/iter +/- 56.00ns ascii::bench_ascii_escape_display_no_escape 91.00ns/iter +/- 1.00ns ```