Don't destructure args tuple in format_args!

[camsteffen]

This allows Clippy to parse the HIR more simply since arg0 is changed to _args.0. (cc rust-lang/rust-clippy#7843). From rustc's perspective, I think this is something between a lateral move and a tiny improvement since there are fewer bindings.

r? @m-ou-se

[camsteffen]

@m-ou-se I picked you as a reviewer for this and #90131 since you wrote the Clippy issue, but feel free to reassign.

This will conflict with rust-lang/rust-clippy#7906. We probably want to merge and sync that PR first so that I can handle the conflict here.

[camsteffen]

@bors try @rust-timer queue

[rust-timer]

Awaiting bors try build completion.

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

[bors]

⌛ Trying commit bf795ffb04b14458dc43234251a7b729d811da08 with merge f629fd83edbea13d29736b99c0725620a9af49f4...

[bors]

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

[rust-timer]

Queued f629fd83edbea13d29736b99c0725620a9af49f4 with parent db062de, future comparison URL.

[rust-timer]

Finished benchmarking commit (f629fd83edbea13d29736b99c0725620a9af49f4): comparison url.

Summary: This change led to small relevant mixed results 🤷 in compiler performance.

• Small improvement in instruction counts (up to -0.2% on incr-unchanged builds of diesel)
• Small regression in instruction counts (up to 0.5% on full builds of deeply-nested-async)

If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf.

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 led to changes in compiler perf.

Next Steps: If you can justify the regressions found in this try perf run, please indicate this with @rustbot label: +perf-regression-triaged along with sufficient written justification. If you cannot justify the regressions please fix the regressions and do another perf run. If the next run shows neutral or positive results, the label will be automatically removed.

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

[m-ou-se]

Looks good to me!

One question: Why _args and not just args?

[camsteffen]

_args is an unused binding in format_args!("just a string"). We could replace match () { _args => [] } with [] in that case but that's a different change. (edit: but I could do that in this PR anyways)

[m-ou-se]

@bors r+

[bors]

📌 Commit bf795ffb04b14458dc43234251a7b729d811da08 has been approved by m-ou-se

[bors]

⌛ Testing commit bf795ffb04b14458dc43234251a7b729d811da08 with merge d4a8cbe2d9cd9d7fd9d57f55c2aa29a4a82a129f...

[bors]

💔 Test failed - checks-actions

[camsteffen]

@bors r=m-ou-se

[bors]

📌 Commit 9f6a58e has been approved by m-ou-se

[bors]

⌛ Testing commit 9f6a58e with merge 71a514c98a4dc779136e50b01e2d85d1529a8ddc...

[bors]

💔 Test failed - checks-actions

[camsteffen]

@flip1995 can I get your approval on that last commit for Clippy?

[flip1995]

Clippy changes LGTM 👍

[camsteffen]

@bors r=m-ou-se

[bors]

📌 Commit 8e21f3a has been approved by m-ou-se

[bors]

⌛ Testing commit 8e21f3a with merge 60952bc...

[bors]

☀️ Test successful - checks-actions
Approved by: m-ou-se
Pushing 60952bc to master...

[rust-timer]

Finished benchmarking commit (60952bc): comparison url.

Summary: This change led to large relevant mixed results 🤷 in compiler performance.

• Large improvement in instruction counts (up to -4.0% on incr-unchanged builds of inflate)
• Large regression in instruction counts (up to 1.3% on incr-patched: println builds of html5ever)

If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf.

Next Steps: If you can justify the regressions found in this perf run, please indicate this with @rustbot label: +perf-regression-triaged along with sufficient written justification. If you cannot justify the regressions please open an issue or create a new PR that fixes the regressions, add a comment linking to the newly created issue or PR, and then add the perf-regression-triaged label to this PR.

@rustbot label: +perf-regression

[rylev]

@camsteffen @m-ou-se this seems to have caused some compiler performance issues. The impacted query seems to be expand_crate which I imagine could be impacted by this change. The change to the html5ever crate is fairly large at 21.8x the significance threshold. Given this change is for a tool that not everyone may use (clippy) but has the potential to impact all compiler workloads, we may want to investigate this further.

That being said, none of the code looked suspicious to me so we may have to run something like cachegrind to really find out. Any ideas before we investigate further?

[camsteffen]

No ideas here. The complexity savings on Clippy's end is rather small so it might not justify the regression.

I've noticed a couple times now that the final automated perf run shows significantly worse results than the manual run. Is there a particular reason for that ?

[rylev]

There wouldn't be any reason for this other than runs are done by fast forwarding the PR's commits on to master and the subsequent changes on master done between when a try run is performed and when the PR is merged can change performance outcomes.

I don't think this is always the case though.