Optimize and improve the proc_macro RPC interface for cross-thread execution #86816
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This greatly improves the performance of the very frequently called `call_site()` macro when running in a cross-thread configuration.
This has minimal impact without changes from a later part, as it also requires support from the ExecutionStrategy. Messages are able to return owning handles without waiting by allocating an ID from the client thread, and passing it up to the server to fill with a response. If the server panics, it will be an ICE, and no further requests will be handled.
…bug flag to use it This new strategy supports avoiding waiting for a reply for noblock messages. This strategy requires using a channel-like approach (similar to the previous CrossThread1 approach). This new CrossThread execution strategy takes a type parameter for the channel to use, allowing rustc to use a more efficient channel which the proc_macro crate could not declare as a dependency.
Compared to mpsc::channel, crossbeam-channel has significantly lower overhead.
… and iterate TokenStreams This significantly reduces the cost of common interactions with TokenStream when running with the CrossThread execution strategy, by reducing the number of RPC calls required.
|
(rust-highfive has picked a reviewer for you, use r? to override) |
rust-highfive
added
the
S-waiting-on-review
This comment has been minimized.
This comment has been minimized.
|
Just to get ahead of this a bit: I was aware that some of this was being worked on by @mystor, and long-term I believe we want all of these changes, but I'm not sure how to approach this PR with all of them at once. If I find enough free time, I could maybe help split into several parts (there's some interesting design changes that aren't strictly tied to cross-thread-specific optimizations, like drastically cutting down on RPC "object" types, keeping interned symbols a strings in the client, etc.) But as of right now I can't promise anything. If someone else is willing to review this in bulk, feel free to go ahead, but even then, I doubt we could land it without an MCP. |
|
I'd be willing to split this into smaller PRs based on the individual patches in the review (each of which should be independent). I unfortunately don't know how to do this on github, however (reading suggests it's not possible). Perhaps we could start with a smaller PR which only includes the changes up to afc36cc, then do the range for the |
This greatly reduces round-trips to fetch relevant extra information about the token in proc macro code, and avoids RPC messages to create Punct tokens.
This greatly reduces round-trips to fetch relevant extra information about the token in proc macro code, and avoids RPC messages to create Group tokens.
This requires a dependency on `unicode-normalization` and `rustc_lexer`, which is currently not possible for `proc_macro`. Instead, a second `extern "C" fn` is provided by the compiler server to perform these steps from any thread. String values are interned in both the server and client, meaning that identifiers can be stringified without any RPC roundtrips without substantially inflating their size. RPC messages passing symbols include the full un-interned value, and are re-interned on the receiving side. This could potentially be optimized in the future. The symbol infrastructure will alwo be used for literals in a following part.
This builds on the symbol infrastructure built for ident to replicate the `LitKind` and `Lit` structures in rustc within the `proc_macro` client, allowing literals to be fully created and interacted with from the client thread. Only parsing and subspan operations still require sync RPC.
|
The job Click to see the possible cause of the failure (guessed by this bot) |
|
Closing this PR for now in favour of #86822 which should provide the |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This series of patches performs various changes to the RPC interface used by
proc_macroto improve its performance. From my local testing (which involved doingcheck --testruns on serde's test suite, based on #49219 (comment)) these pages have small improvements to same-thread execution, and significant improvements to cross-thread execution.Some of the major changes include:
Span::{def,call,mixed}_site) in the client bridge, so no RPC is required to access them,TokenStream::clone()or composingToken{Tree,Stream}s together),TokenStreamoperations (removesTokenStreamBuilder,TokenStreamIterin favour of passingVec<TokenTree>over RPC),Option<bridge::client::TokenStream>in theTokenStreamtype to avoidTokenStream::new()call overhead,Symbols in the client, along with support to serialize them over RPC,Group,Punct,Ident, andLiteralinto struct types around more primitive handles (TokenStream,Symbol, andSpan), meaning that common operations on these types, such as accessing.span()or.delimiter()require no RPC calls,crossbeam-channelfor cross-thread message passing, to avoidmpsc::channel's overhead,-Z proc_macro_cross_thread) to turn on theCrossThreadexecution strategy for proc macros without rebuilding.The PR is organized into a sequence of commits which build on top of one another, and could be reviewed independently.
Some example times from my machine (there's unfortunately variation in times run-to-run, but I tried to pick representative examples):
PR (SameThread)
PR (CrossThread)
HEAD (SameThread)
HEAD (CrossThread2)
(note: there will likely be changes required to RA to support these changes as well, however this patch currently includes no changes to RA or it's copy of the
proc_macrocrate)