proc_macro/bridge: Eagerly pass TokenStream contents into the client

[mystor]

This change could improve performance, especially in cases like with the
cross-thread backend which have expensive RPC. It handles this by moving
more of the logic for TokenStream into the client, storing a
Rc<Vec<TokenTree>> directly, rather than converting to/from the
compiler representation over RPC for every change.

As this eagerly decomposes interpolated AST fragments, this ended up
changing the output of TokenStream::to_string(). To keep up
compatibility for now, formatting is still performed using RPC and
rustc_ast_pretty, however in the future we may look into decoupling
proc-macro formatting from rustc_ast_pretty, and doing it in the
client.

One side-effect of this was that the workaround for the
procedural-masquerade crate had to be updated to work with the new
approach, as we couldn't depend on the AST fragment printer to format
the input. To do this, a real , token is now inserted when passing the
enum to a proc-macro rather than depending on the pretty-printing
behaviour. From local tests it appears that the new output should be
compatible enough to work with older versions of procedural-masquerade.

The new conversion between the proc-macro and compiler representations
of a TokenStream is now handled by the RpcContext type provided by
the proc-macro server. This type is a distinct value which will be
fetched and stored by the server before commands are run, such that the
decoders can have access to relevant state. In the future, we may also
change the symbol handling code to work off of RpcContext rather than
being static methods on the Server trait.

[rust-highfive]

r? @petrochenkov

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

[mystor]

Posting this change as a work-in-progress change, which is not ready to merge. I think we'll want to run this through crater before we move too much further with it (assuming it passes CI), to make sure it doesn't cause breakage in the ecosystem.

r? @eddyb

[mystor]

Another potential concern here is that the code for serializing and deserializing TokenStream with this patch is written in a recursive manner, as is the stringification/formatting code. I think the formatting code previously (from rustc_ast_pretty) was also recursive, but this could theoretically exhaust the stack for sufficiently nested TokenTrees.

That being said, it should be possible to rework things to avoid using the stack in the future, but keeping it this way for now makes everything much simpler.

[eddyb]

Might be too early for this, but curious anyway:
@bors try @rust-timer queue

[rust-timer]

Awaiting bors try build completion.

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

[bors]

⌛ Trying commit 7a64bb64f92fa2c6741c3b4938804514e46b0097 with merge 0c7f4114a5582dfa18524643326a0e87bc6d62f3...

[bors]

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

[rust-timer]

Queued 0c7f4114a5582dfa18524643326a0e87bc6d62f3 with parent a2cdcb3, future comparison URL.

[mystor]

Looks like I missed 2 proc macros in tests with changes to their output formats. Fortunately shouldn't be hard to update them.

[mystor]

It occurred to me that pretty-printing a large TokenStream might be rare enough nowadays that we can afford sending the entire TokenStream over IPC just to pretty-print it, at least as a stopgap, to avoid some of the pretty-printing changes from also happening in this patch.

I still have the other changes to do it more efficiently, but we can potentially leave that for a follow-up if the perf on this approach is OK.

This still requires the changes to the pretty-printing hack for procedural-masquerade because the interpolated node can't be used anymore.

[mystor]

cc #73933 (@Aaron1011 @petrochenkov), as that issue tracks removing the hack completely.

[rust-timer]

Finished benchmarking commit (0c7f4114a5582dfa18524643326a0e87bc6d62f3): comparison URL.

Overall result: ❌✅ regressions and improvements - 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.

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

Instruction count

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

	mean1	range	count2
Regressions ❌ (primary)	1.2%	[0.3%, 4.3%]	35
Regressions ❌ (secondary)	0.9%	[0.7%, 1.0%]	4
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-15.9%	[-43.7%, -1.0%]	11
All ❌✅ (primary)	1.2%	[0.3%, 4.3%]	35

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.

	mean1	range	count2
Regressions ❌ (primary)	4.3%	[4.3%, 4.3%]	1
Regressions ❌ (secondary)	3.0%	[2.6%, 3.5%]	2
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-3.0%	[-3.9%, -2.1%]	2
All ❌✅ (primary)	4.3%	[4.3%, 4.3%]	1

Cycles

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.

	mean1	range	count2
Regressions ❌ (primary)	3.0%	[1.4%, 6.7%]	8
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-12.5%	[-31.7%, -2.2%]	10
All ❌✅ (primary)	3.0%	[1.4%, 6.7%]	8

Footnotes

1. the arithmetic mean of the percent change ↩ ↩² ↩³

2. number of relevant changes ↩ ↩² ↩³

[mystor]

These results so far make sense. The runtime performance is better (as noticed by e.g. token-stream-stress which improved check by 43%), but I haven't manually optimized away inlining opportunities in the Extend and FromIterator implementations, so the optimizer spends substantially more time on calls to those functions in crates like syn and serde-derive, slowing down those builds. Once I've minimized the generic code in those functions those regressions should hopefully go away a bit.

[bors]

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

[Dylan-DPC]

@mystor any updates on this?

[mystor]

No, I unfortunately haven't had the time to dig into fixing this patch up & getting it ready to be looked into more.

[Dylan-DPC]

Closing this as inactive. Feel free to reöpen this pr or create a new pr if you get the time to work on this. Thanks