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

Write char::DebugEscape sequences using write_str #124575

Closed
wants to merge 2 commits into from

Conversation

Swatinem
Copy link
Contributor

@Swatinem Swatinem commented May 1, 2024

Instead of writing each char of an escape sequence one by one,
this delegates to Display, which uses write_str internally
in order to write the whole escape sequence at once.


This is on top of #124551, and looking at the individual commit, you can see the improvements to the write_calls

In order to inform future perf improvements and prevent regressions,
lets add some benchmarks that stress `impl Debug for str`.
Instead of writing each `char` of an escape sequence one by one,
this delegates to `Display`, which uses `write_str` internally
in order to write the whole escape sequence at once.
@rustbot
Copy link
Collaborator

rustbot commented May 1, 2024

r? @workingjubilee

rustbot has assigned @workingjubilee.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@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 May 1, 2024
@Swatinem
Copy link
Contributor Author

Swatinem commented May 1, 2024

r? @the8472
as you are already reviewing #124551 :-)

@rustbot rustbot assigned the8472 and unassigned workingjubilee May 1, 2024
@Swatinem
Copy link
Contributor Author

Swatinem commented May 1, 2024

Even though I’m far from following the best practices for benchmarking, I seem to get quite a consistent improvement from this, even for the benchmarks that are not doing any escaping, possibly because this generates better/smaller code overall?

 name                        master ns/iter  write-str ns/iter  diff ns/iter   diff %  speedup 
 str::debug::ascii_escapes   1,083           1,003                       -80   -7.39%   x 1.08
 str::debug::ascii_only      700             672                         -28   -4.00%   x 1.04
 str::debug::mixed           1,748           1,549                      -199  -11.38%   x 1.13
 str::debug::mostly_unicode  1,250           1,176                       -74   -5.92%   x 1.06
 str::debug::some_unicode    748             717                         -31   -4.14%   x 1.04

@bors
Copy link
Contributor

bors commented May 24, 2024

☔ The latest upstream changes (presumably #121150) made this pull request unmergeable. Please resolve the merge conflicts.

@Swatinem
Copy link
Contributor Author

landed as part of #121150

@Swatinem Swatinem closed this May 24, 2024
@Swatinem Swatinem deleted the debug-str-write-once branch May 24, 2024 21:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants