Skip to content
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

Merged
merged 1 commit into from
Jan 20, 2024

Conversation

the8472
Copy link
Member

@the8472 the8472 commented Jun 29, 2023

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

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
@rustbot
Copy link
Collaborator

rustbot commented Jun 29, 2023

r? @m-ou-se

(rustbot has picked a reviewer for you, use r? to override)

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Jun 29, 2023
@Mark-Simulacrum
Copy link
Member

@bors r+

r? @Mark-Simulacrum

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...

@bors
Copy link
Contributor

bors commented Jan 20, 2024

📌 Commit 6c87448 has been approved by Mark-Simulacrum

It is now in the queue for this repository.

@rustbot rustbot assigned Mark-Simulacrum and unassigned m-ou-se Jan 20, 2024
@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jan 20, 2024
bors added a commit to rust-lang-ci/rust that referenced this pull request Jan 20, 2024
…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
Comment on lines +27 to +28

pub(crate) fn into_inner(self) -> Option<I> {
Copy link
Contributor

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?

Copy link
Member Author

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.

Comment on lines +71 to +72

pub(crate) fn into_inner(self) -> I {
Copy link
Contributor

Choose a reason for hiding this comment

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

here too.

bors added a commit to rust-lang-ci/rust that referenced this pull request Jan 20, 2024
…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
@bors bors merged commit 17c95b6 into rust-lang:master Jan 20, 2024
@rustbot rustbot added this to the 1.77.0 milestone Jan 20, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Jan 20, 2024
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
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants