remove some indirection from proc_macro_server

[mystor]

This is a theoretical fix for the performance regression from #87264, based on wild guessing about what may have caused the regression. The patch reduces indirections when getting a ParseSess and debug-formatting Spans in the proc macro server.

[rust-highfive]

r? @oli-obk

(rust-highfive has picked a reviewer for you, use r? to override)

[oli-obk]

@bors try @rust-timer queue

[rust-timer]

Awaiting bors try build completion.

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

[bors]

⌛ Trying commit 943798401d23b0dfedfa3705aa6744bf5ec6cf1f with merge 16d626fda14734fe3545bd55d283c5dd2973d0e8...

[bors]

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

[rust-timer]

Queued 16d626fda14734fe3545bd55d283c5dd2973d0e8 with parent 1b12d01, future comparison URL.

[rust-timer]

Finished benchmarking commit (16d626fda14734fe3545bd55d283c5dd2973d0e8): comparison url.

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

• Large improvement in instruction counts (up to -3.8% on incr-unchanged builds of deep-vector)
• Moderate regression in instruction counts (up to 0.5% on incr-unchanged builds of style-servo)

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

[mystor]

Well, that appears to have changed perf counts, but for a completely different test than the regression in #87264 (comment)...

[oli-obk]

I don't think this should cause any perf change, can you test it in isolation?

[oli-obk]

maybe we'll get the same perf if parse_sess and sess are both #[inline]?

[mystor]

Perhaps. My reasoning here was just that it would remove some pointer chasing, as what was previously 1 indirect read (self.sess) is now 2 (self.ecx.sess, parse_sess is an inline field of Session, so doesn't require an additional pointer chase). If the functions are not being inlined, though, that's more likely to be the cause of the perf loss.

I'll push another version where I just #[inline] the two functions.

[mystor]

I'm guessing I don't have the permissions to ask bors to run perf tests, but I suppose it's worth a try :-)

@bors try @rust-timer queue

[rust-timer]

Insufficient permissions to issue commands to rust-timer.

[bors]

@mystor: 🔑 Insufficient privileges: not in try users

[oli-obk]

@bors try @rust-timer queue

[rust-timer]

Awaiting bors try build completion.

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

[bors]

⌛ Trying commit e168f91 with merge 412e53deb3152c35a44186f969f8334da38f3d35...

[bors]

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

[rust-timer]

Queued 412e53deb3152c35a44186f969f8334da38f3d35 with parent 41301c3, future comparison URL.

[rust-timer]

Finished benchmarking commit (412e53deb3152c35a44186f969f8334da38f3d35): comparison url.

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

• Small improvement in instruction counts (up to -1.0% on incr-full builds of inflate)
• Small regression in instruction counts (up to 0.5% on incr-unchanged builds of helloworld)

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

[oli-obk]

do you want to try something else here or should we just close? Doesn't seem like there is anything obvious

[mystor]

Yeah I don't see anything obvious and don't have a ton of time to continue investigating right now, so closing the bug seems reasonable to me.