Use write_str() instead of write!() when possible

[kornelski]

write!(formatter, "literal") needlessly constructs fmt::Arguments::new_v1 that doesn't fully optimize out. OTOH formatter.write_str("literal") compiles to just a function call, so it's quicker to compile and results in smaller binaries.

I took care to correct uses of {{ and }} in the formatting string.

[rustbot]

r? @lcnr

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

[rustbot]

rustc_macros::diagnostics was changed

cc @davidtwco, @compiler-errors, @JohnTitor, @TaKO8Ki

Some changes occurred to the CTFE / Miri engine

cc @rust-lang/miri

Some changes occurred to the CTFE / Miri engine

cc @rust-lang/miri

rustc_error_messages was changed

cc @davidtwco, @compiler-errors, @JohnTitor, @TaKO8Ki

Hey! It looks like you've submitted a new PR for the library teams!

If this PR contains changes to any rust-lang/rust public library APIs then please comment with @rustbot label +T-libs-api -T-libs to tag it appropriately. If this PR contains changes to any unstable APIs please edit the PR description to add a link to the relevant API Change Proposal or create one if you haven't already. If you're unsure where your change falls no worries, just leave it as is and the reviewer will take a look and make a decision to forward on if necessary.

Examples of T-libs-api changes:

• Stabilizing library features
• Introducing insta-stable changes such as new implementations of existing stable traits on existing stable types
• Introducing new or changing existing unstable library APIs (excluding permanently unstable features / features without a tracking issue)
• Changing public documentation in ways that create new stability guarantees
• Changing observable runtime behavior of library APIs

[saethlin]

Does the optimization difference disappear with #106824 ?

It seems like the existing style is the opposite of this PR. Can/should we have a tidy check to enforce this style?

[RalfJung]

Unless benchmarks show that this is actually perf-critical, I'd rather stick to the old code and prioritize code readability and uniformity. It's better (IMO) to have the same uniform code for format strings with and without any interpolation.

[Kobzol]

@bors try @rust-timer queue

Let's see the perf. effect of this. I also wonder whether this is required after Mara's inlining PR.

[bors]

⌛ Trying commit e867567 with merge f48221e382b2eb037dcb7d7de468168f7069c810...

[kornelski]

#106824 is nice, but does not address the same thing. It simplifies Arguments passed to the formatter. This PR goes further skips the dynamic part of the formatting and prevents Arguments from being created in the first place. Perhaps on the scale of the whole compiler that's a tiny difference, but on the scale of string-formatting code it halves number of instructions.

The closure-based rewrite of formatting #101568 can achieve the same result in optimized code.

[bors]

☀️ Try build successful - checks-actions
Build commit: f48221e382b2eb037dcb7d7de468168f7069c810 (f48221e382b2eb037dcb7d7de468168f7069c810)

[m-ou-se]

write!(formatter, "literal") needlessly constructs fmt::Arguments::new_v1 that doesn't fully optimize out.

Hm, it shouldn't be too hard to fix that.

[m-ou-se]

I've added it to the list in #99012:

• Make sure write!(f, "literal") is just as efficient as f.write_str("literal")

[oli-obk]

@rust-timer build f48221e382b2eb037dcb7d7de468168f7069c810

let's still have a look at perf so we can add the data to the tracking issue, but close this PR once perf is done.

[oli-obk]

oh perf is just swamped. will take a bit to get a result here

[rust-timer]

Finished benchmarking commit (f48221e382b2eb037dcb7d7de468168f7069c810): comparison URL.

Overall result: ✅ improvements - no action needed

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR may lead to changes in compiler perf.

@bors rollup=never
@rustbot label: -S-waiting-on-perf -perf-regression

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-0.9%	[-1.3%, -0.5%]	8
All ❌✅ (primary)	-	-	0

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

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

Cycles

This benchmark run did not return any relevant results for this metric.

[oli-obk]

Looks neutral with some smaller wins that may be noise.

Changing this isn't worth it in the compiler, but cleaning it up in general is still something to explore

[Kobzol]

I think that the wins look like more than noise. But I agree that it's probably not worth to merge this, as the wins are not big enough to justify the "worse/inconsistent" code style. Especially if there's hope that Mara's changes can win the perf back here soon-ish.

[lqd]

I also believe the tt-muncher wins are noise: here's how it currently looks

[Kobzol]

I was looking at the non-relevant results and it seemed to me that there are a lot of green results. In any case, the wins are definitely small, that's true :)