[perf] Avoid checking for alternate Debug on every struct field.

[obi1kenobi]

The fmt-debug-derive benchmark shows there's a -6.78% instruction win to be had by not checking for alternate ("pretty") formatting for every field in the pre-written 1-5 arity cases of derived Debug struct representations.

Since non-alternate mode is the default, add a fast path that supports only checking for alternate mode once total per formatting invocation, rather than once per field per invocation.

It's likely that the analogous methods for tuple structs would benefit from a similar change, but the fmt-debug-derive benchmark doesn't include this case. If the reviewers are amenable to it, I'd be happy to add both a new benchmark and, if perf-positive, make the analogous tuple struct optimization.

r? @Kobzol

AI disclosure: The optimization opportunity here was discovered as part of a systematic probe for missed optimizations utilizing both traditional and AI tools. The code here was initially prototyped and vetted by AI tools, followed by additional manual work. I secured approval in advance from the reviewer I pinged. I stand behind the quality of the code I'm submitting, and I vouch it's as good or better compared to if I had written every line by my own hand.

[rustbot]

kobzol is not on the review rotation at the moment.
They may take a while to respond.

[obi1kenobi]

@rustbot r? @Kobzol @joshtriplett

[rustbot]

Requested reviewer is already assigned to this pull request.

Please choose another assignee.

[Kobzol]

@bors try @rust-timer queue

[rust-bors]

☀️ Try build successful (CI)
Build commit: ebaa02e (ebaa02e6d9bfb6b35b6e45eaccab8b7ebb9c6461)
Base parent: 48e8ec6 (48e8ec6f05057f942a97187dde770a118e05f42c)

[kn1g78]

Could we add a regression test covering derived named structs with 1–5 fields under both {:?} and {:#?}, since the existing focused test only covers two fields?

View changes since the review

[obi1kenobi]

Yes, happy to add the test. Just wanted to make sure fmt-debug-derive did get faster as I expected.

Will add it probably tonight.

[obi1kenobi]

Done! Apologies for the wait. Also updated the PR description with an observation about an analogous optimization for tuple structs.

[rust-timer]

Finished benchmarking commit (ebaa02e): comparison URL.

Overall result: no relevant changes - no action needed

Benchmarking means the PR may be perf-sensitive. Consider adding rollup=never if this change is not fit for rolling up.

@rustbot label: -S-waiting-on-perf -perf-regression

Instruction count

This perf run didn't have relevant results for this metric.

Max RSS (memory usage)

Results (primary 3.1%, secondary 2.6%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	3.1%	[3.1%, 3.1%]	1
Regressions ❌ (secondary)	2.6%	[2.3%, 3.0%]	2
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	3.1%	[3.1%, 3.1%]	1

Cycles

Results (secondary -0.8%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	3.1%	[3.1%, 3.1%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-2.7%	[-3.1%, -2.4%]	2
All ❌✅ (primary)	-	-	0

Binary size

Results (primary 0.0%, secondary 0.0%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	0.0%	[0.0%, 0.0%]	4
Regressions ❌ (secondary)	0.0%	[0.0%, 0.0%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	0.0%	[0.0%, 0.0%]	4

Bootstrap: 490.729s -> 490.282s (-0.09%)
Artifact size: 389.29 MiB -> 389.24 MiB (-0.01%)

[obi1kenobi]

fmt-debug-derive shows a -6.78% improvement in wall-clock time: https://perf.rust-lang.org/compare.html?start=48e8ec6f05057f942a97187dde770a118e05f42c&end=ebaa02e6d9bfb6b35b6e45eaccab8b7ebb9c6461&stat=instructions%3Au&tab=runtime

I'm still learning the ropes of how perf-testing is set up, and I was a bit surprised that we don't seem to have fmt-debug-derive in the instruction-counting benchmarks too. Is there intuition on when we check only wall-clock runtime vs also checking instruction counts?

[panstromek]

What you're linking is instruction count results and fmt-debug-derive is there, I assume you meant the opposite? But the benchmark is in wall-clock, too, it's just not visible by default, because it didn't meet the significance treshold. If you tick the 'Show non-relevant results' checkbox, you can see all results, where it shows up.

[obi1kenobi]

Oops, I was looking at the compile time tab 🤦

[rustbot]

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

[obi1kenobi]

Updated with test cases and doc comments, but leaving the implementation identical to the benchmarked code.

[rust-bors]

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

[Kobzol]

Hmm, it's a bit suspicious that removing just a single condition that performs a very simple operation, and I'd expect it would be almost perfectly branch-predicted, has such an effect. I tested it locally with cachegrind in rustc-perf:

$ cargo run --bin collector profile_runtime cachegrind `rustup +48e8ec6f05057f942a97187dde770a118e05f42c which rustc` --rustc2 `rustup +ebaa02e6d9bfb6b35b6e45eaccab8b7ebb9c6461 which rustc` fmt-debug-derive --group fmt`

and it seems that there is a difference in the field function not being called:

< -128,000,000  library/core/src/fmt/builders.rs:
  -123,000,000    <core::fmt::builders::DebugStruct>::field
    -5,000,000    <core::fmt::Formatter>::debug_struct_field3_finish

<   14,000,000  library/core/src/fmt/mod.rs:
    53,000,000    <core::fmt::Formatter>::debug_struct_field3_finish
   -39,000,000    <core::fmt::builders::DebugStruct>::field

<    3,000,000  library/core/src/result.rs:
    30,000,000    <core::fmt::Formatter>::debug_struct_field3_finish
   -27,000,000    <core::fmt::builders::DebugStruct>::field

I wonder if this is caused more by newly added inlining, rather than removing the if condition. Just out of curiosity, I tried added forced inlining (though probably it won't work like I expect it to) in #112049, to see if that has a similar effect.

[Kobzol]

Hmm, yeah, this kind of inlining didn't really have any effect.

@nnethercote Could you please take a look at this, since you did some work on optimizing these debug builders previously, IIRC? It looks reasonable to me, though I'm not sure how large would the effect be in real-world programs.