-
Notifications
You must be signed in to change notification settings - Fork 13k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Optimize and improve the proc_macro RPC interface for cross-thread execution #86816
Conversation
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) |
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 |
This series of patches performs various changes to the RPC interface used by
proc_macro
to improve its performance. From my local testing (which involved doingcheck --test
runs 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),TokenStream
operations (removesTokenStreamBuilder
,TokenStreamIter
in favour of passingVec<TokenTree>
over RPC),Option<bridge::client::TokenStream>
in theTokenStream
type to avoidTokenStream::new()
call overhead,Symbol
s in the client, along with support to serialize them over RPC,Group
,Punct
,Ident
, andLiteral
into 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-channel
for cross-thread message passing, to avoidmpsc::channel
's overhead,-Z proc_macro_cross_thread
) to turn on theCrossThread
execution 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_macro
crate)