Skip to content

Commit 8315ce9

Browse files
committed
proc_macro: cache static spans in client's thread-local state
This greatly improves the performance of the very frequently called `call_site()` macro when running in a cross-thread configuration.
1 parent cc698dc commit 8315ce9

File tree

6 files changed

+169
-98
lines changed

6 files changed

+169
-98
lines changed

compiler/rustc_expand/src/proc_macro_server.rs

Lines changed: 18 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -375,7 +375,10 @@ impl<'a, 'b> Rustc<'a, 'b> {
375375
}
376376

377377
fn lit(&mut self, kind: token::LitKind, symbol: Symbol, suffix: Option<Symbol>) -> Literal {
378-
Literal { lit: token::Lit::new(kind, symbol, suffix), span: server::Span::call_site(self) }
378+
Literal {
379+
lit: token::Lit::new(kind, symbol, suffix),
380+
span: server::Context::call_site(self),
381+
}
379382
}
380383
}
381384

@@ -528,7 +531,7 @@ impl server::Group for Rustc<'_, '_> {
528531
Group {
529532
delimiter,
530533
stream,
531-
span: DelimSpan::from_single(server::Span::call_site(self)),
534+
span: DelimSpan::from_single(server::Context::call_site(self)),
532535
flatten: false,
533536
}
534537
}
@@ -554,7 +557,7 @@ impl server::Group for Rustc<'_, '_> {
554557

555558
impl server::Punct for Rustc<'_, '_> {
556559
fn new(&mut self, ch: char, spacing: Spacing) -> Self::Punct {
557-
Punct::new(ch, spacing == Spacing::Joint, server::Span::call_site(self))
560+
Punct::new(ch, spacing == Spacing::Joint, server::Context::call_site(self))
558561
}
559562
fn as_char(&mut self, punct: Self::Punct) -> char {
560563
punct.ch
@@ -777,15 +780,6 @@ impl server::Span for Rustc<'_, '_> {
777780
format!("{:?} bytes({}..{})", span.ctxt(), span.lo().0, span.hi().0)
778781
}
779782
}
780-
fn def_site(&mut self) -> Self::Span {
781-
self.def_site
782-
}
783-
fn call_site(&mut self) -> Self::Span {
784-
self.call_site
785-
}
786-
fn mixed_site(&mut self) -> Self::Span {
787-
self.mixed_site
788-
}
789783
fn source_file(&mut self, span: Self::Span) -> Self::SourceFile {
790784
self.sess().source_map().lookup_char_pos(span.lo()).file
791785
}
@@ -861,3 +855,15 @@ impl server::Span for Rustc<'_, '_> {
861855
})
862856
}
863857
}
858+
859+
impl server::Context for Rustc<'_, '_> {
860+
fn def_site(&mut self) -> Self::Span {
861+
self.def_site
862+
}
863+
fn call_site(&mut self) -> Self::Span {
864+
self.call_site
865+
}
866+
fn mixed_site(&mut self) -> Self::Span {
867+
self.mixed_site
868+
}
869+
}

library/proc_macro/src/bridge/client.rs

Lines changed: 85 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -241,6 +241,20 @@ impl Clone for SourceFile {
241241
}
242242
}
243243

244+
impl Span {
245+
pub(crate) fn def_site() -> Span {
246+
Bridge::with(|bridge| bridge.context.def_site)
247+
}
248+
249+
pub(crate) fn call_site() -> Span {
250+
Bridge::with(|bridge| bridge.context.call_site)
251+
}
252+
253+
pub(crate) fn mixed_site() -> Span {
254+
Bridge::with(|bridge| bridge.context.mixed_site)
255+
}
256+
}
257+
244258
impl fmt::Debug for Span {
245259
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
246260
f.write_str(&self.debug())
@@ -274,6 +288,21 @@ macro_rules! define_client_side {
274288
}
275289
with_api!(self, self, define_client_side);
276290

291+
struct Bridge<'a> {
292+
/// Reusable buffer (only `clear`-ed, never shrunk), primarily
293+
/// used for making requests.
294+
cached_buffer: Buffer,
295+
296+
/// Server-side function that the client uses to make requests.
297+
dispatch: closure::Closure<'a, Buffer, Buffer>,
298+
299+
/// Provided context for this macro expansion.
300+
context: ExpnContext<Span>,
301+
}
302+
303+
impl<'a> !Send for Bridge<'a> {}
304+
impl<'a> !Sync for Bridge<'a> {}
305+
277306
enum BridgeState<'a> {
278307
/// No server is currently connected to this client.
279308
NotConnected,
@@ -316,34 +345,6 @@ impl BridgeState<'_> {
316345
}
317346

318347
impl Bridge<'_> {
319-
pub(crate) fn is_available() -> bool {
320-
BridgeState::with(|state| match state {
321-
BridgeState::Connected(_) | BridgeState::InUse => true,
322-
BridgeState::NotConnected => false,
323-
})
324-
}
325-
326-
fn enter<R>(self, f: impl FnOnce() -> R) -> R {
327-
let force_show_panics = self.force_show_panics;
328-
// Hide the default panic output within `proc_macro` expansions.
329-
// NB. the server can't do this because it may use a different libstd.
330-
static HIDE_PANICS_DURING_EXPANSION: Once = Once::new();
331-
HIDE_PANICS_DURING_EXPANSION.call_once(|| {
332-
let prev = panic::take_hook();
333-
panic::set_hook(Box::new(move |info| {
334-
let show = BridgeState::with(|state| match state {
335-
BridgeState::NotConnected => true,
336-
BridgeState::Connected(_) | BridgeState::InUse => force_show_panics,
337-
});
338-
if show {
339-
prev(info)
340-
}
341-
}));
342-
});
343-
344-
BRIDGE_STATE.with(|state| state.set(BridgeState::Connected(self), f))
345-
}
346-
347348
fn with<R>(f: impl FnOnce(&mut Bridge<'_>) -> R) -> R {
348349
BridgeState::with(|state| match state {
349350
BridgeState::NotConnected => {
@@ -357,6 +358,13 @@ impl Bridge<'_> {
357358
}
358359
}
359360

361+
pub(crate) fn is_available() -> bool {
362+
BridgeState::with(|state| match state {
363+
BridgeState::Connected(_) | BridgeState::InUse => true,
364+
BridgeState::NotConnected => false,
365+
})
366+
}
367+
360368
/// A client-side RPC entry-point, which may be using a different `proc_macro`
361369
/// from the one used by the server, but can be invoked compatibly.
362370
///
@@ -374,7 +382,7 @@ pub struct Client<I, O> {
374382
// a wrapper `fn` pointer, once `const fn` can reference `static`s.
375383
pub(super) get_handle_counters: extern "C" fn() -> &'static HandleCounters,
376384

377-
pub(super) run: extern "C" fn(Bridge<'_>) -> Buffer,
385+
pub(super) run: extern "C" fn(BridgeConfig<'_>) -> Buffer,
378386

379387
pub(super) _marker: PhantomData<fn(I) -> O>,
380388
}
@@ -386,40 +394,62 @@ impl<I, O> Clone for Client<I, O> {
386394
}
387395
}
388396

397+
fn maybe_install_panic_hook(force_show_panics: bool) {
398+
// Hide the default panic output within `proc_macro` expansions.
399+
// NB. the server can't do this because it may use a different libstd.
400+
static HIDE_PANICS_DURING_EXPANSION: Once = Once::new();
401+
HIDE_PANICS_DURING_EXPANSION.call_once(|| {
402+
let prev = panic::take_hook();
403+
panic::set_hook(Box::new(move |info| {
404+
let show = BridgeState::with(|state| match state {
405+
BridgeState::NotConnected => true,
406+
BridgeState::Connected(_) | BridgeState::InUse => force_show_panics,
407+
});
408+
if show {
409+
prev(info)
410+
}
411+
}));
412+
});
413+
}
414+
389415
/// Client-side helper for handling client panics, entering the bridge,
390416
/// deserializing input and serializing output.
391417
// FIXME(eddyb) maybe replace `Bridge::enter` with this?
392418
fn run_client<A: for<'a, 's> DecodeMut<'a, 's, ()>, R: Encode<()>>(
393-
mut bridge: Bridge<'_>,
419+
config: BridgeConfig<'_>,
394420
f: impl FnOnce(A) -> R,
395421
) -> Buffer {
396-
// The initial `cached_buffer` contains the input.
397-
let mut buf = bridge.cached_buffer.take();
422+
let BridgeConfig { input: mut buf, dispatch, force_show_panics, .. } = config;
398423

399424
panic::catch_unwind(panic::AssertUnwindSafe(|| {
400-
bridge.enter(|| {
401-
let reader = &mut &buf[..];
402-
let input = A::decode(reader, &mut ());
403-
404-
// Put the `cached_buffer` back in the `Bridge`, for requests.
405-
Bridge::with(|bridge| bridge.cached_buffer = buf.take());
406-
407-
let output = f(input);
408-
409-
// Take the `cached_buffer` back out, for the output value.
410-
buf = Bridge::with(|bridge| bridge.cached_buffer.take());
411-
412-
// HACK(eddyb) Separate encoding a success value (`Ok(output)`)
413-
// from encoding a panic (`Err(e: PanicMessage)`) to avoid
414-
// having handles outside the `bridge.enter(|| ...)` scope, and
415-
// to catch panics that could happen while encoding the success.
416-
//
417-
// Note that panics should be impossible beyond this point, but
418-
// this is defensively trying to avoid any accidental panicking
419-
// reaching the `extern "C"` (which should `abort` but might not
420-
// at the moment, so this is also potentially preventing UB).
421-
buf.clear();
422-
Ok::<_, ()>(output).encode(&mut buf, &mut ());
425+
maybe_install_panic_hook(force_show_panics);
426+
427+
let reader = &mut &buf[..];
428+
let (input, context) = <(A, ExpnContext<Span>)>::decode(reader, &mut ());
429+
430+
// Put the buffer we used for input back in the `Bridge` for requests.
431+
let new_state =
432+
BridgeState::Connected(Bridge { cached_buffer: buf.take(), dispatch, context });
433+
434+
BRIDGE_STATE.with(|state| {
435+
state.set(new_state, || {
436+
let output = f(input);
437+
438+
// Take the `cached_buffer` back out, for the output value.
439+
buf = Bridge::with(|bridge| bridge.cached_buffer.take());
440+
441+
// HACK(eddyb) Separate encoding a success value (`Ok(output)`)
442+
// from encoding a panic (`Err(e: PanicMessage)`) to avoid
443+
// having handles outside the `bridge.enter(|| ...)` scope, and
444+
// to catch panics that could happen while encoding the success.
445+
//
446+
// Note that panics should be impossible beyond this point, but
447+
// this is defensively trying to avoid any accidental panicking
448+
// reaching the `extern "C"` (which should `abort` but might not
449+
// at the moment, so this is also potentially preventing UB).
450+
buf.clear();
451+
Ok::<_, ()>(output).encode(&mut buf, &mut ());
452+
})
423453
})
424454
}))
425455
.map_err(PanicMessage::from)

library/proc_macro/src/bridge/mod.rs

Lines changed: 20 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -155,9 +155,6 @@ macro_rules! with_api {
155155
},
156156
Span {
157157
fn debug($self: $S::Span) -> String;
158-
fn def_site() -> $S::Span;
159-
fn call_site() -> $S::Span;
160-
fn mixed_site() -> $S::Span;
161158
fn source_file($self: $S::Span) -> $S::SourceFile;
162159
fn parent($self: $S::Span) -> Option<$S::Span>;
163160
fn source($self: $S::Span) -> $S::Span;
@@ -217,16 +214,15 @@ use buffer::Buffer;
217214
pub use rpc::PanicMessage;
218215
use rpc::{Decode, DecodeMut, Encode, Reader, Writer};
219216

220-
/// An active connection between a server and a client.
221-
/// The server creates the bridge (`Bridge::run_server` in `server.rs`),
222-
/// then passes it to the client through the function pointer in the `run`
223-
/// field of `client::Client`. The client holds its copy of the `Bridge`
217+
/// Configuration for establishing an active connection between a server and a
218+
/// client. The server creates the bridge config (`run_server` in `server.rs`),
219+
/// then passes it to the client through the function pointer in the `run` field
220+
/// of `client::Client`. The client constructs a local `Bridge` from the config
224221
/// in TLS during its execution (`Bridge::{enter, with}` in `client.rs`).
225222
#[repr(C)]
226-
pub struct Bridge<'a> {
227-
/// Reusable buffer (only `clear`-ed, never shrunk), primarily
228-
/// used for making requests, but also for passing input to client.
229-
cached_buffer: Buffer,
223+
pub struct BridgeConfig<'a> {
224+
/// Buffer used to pass initial input to the client.
225+
input: Buffer,
230226

231227
/// Server-side function that the client uses to make requests.
232228
dispatch: closure::Closure<'a, Buffer, Buffer>,
@@ -477,3 +473,16 @@ compound_traits!(
477473
Literal(tt),
478474
}
479475
);
476+
477+
/// Context provided alongside the initial inputs for a macro expansion.
478+
/// Provides values such as spans which are used frequently to avoid RPC.
479+
#[derive(Clone)]
480+
struct ExpnContext<S> {
481+
def_site: S,
482+
call_site: S,
483+
mixed_site: S,
484+
}
485+
486+
compound_traits!(
487+
struct ExpnContext<Sp> { def_site, call_site, mixed_site }
488+
);

library/proc_macro/src/bridge/selfless_reify.rs

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -75,9 +75,10 @@ macro_rules! define_reify_functions {
7575
define_reify_functions! {
7676
fn _reify_to_extern_c_fn_unary<A, R> for extern "C" fn(arg: A) -> R;
7777

78-
// HACK(eddyb) this abstraction is used with `for<'a> fn(Bridge<'a>) -> T`
79-
// but that doesn't work with just `reify_to_extern_c_fn_unary` because of
80-
// the `fn` pointer type being "higher-ranked" (i.e. the `for<'a>` binder).
81-
// FIXME(eddyb) try to remove the lifetime from `Bridge`, that'd help.
82-
fn reify_to_extern_c_fn_hrt_bridge<R> for extern "C" fn(bridge: super::Bridge<'_>) -> R;
78+
// HACK(eddyb) this abstraction is used with `for<'a> fn(BridgeConfig<'a>)
79+
// -> T` but that doesn't work with just `reify_to_extern_c_fn_unary`
80+
// because of the `fn` pointer type being "higher-ranked" (i.e. the
81+
// `for<'a>` binder).
82+
// FIXME(eddyb) try to remove the lifetime from `BridgeConfig`, that'd help.
83+
fn reify_to_extern_c_fn_hrt_bridge<R> for extern "C" fn(bridge: super::BridgeConfig<'_>) -> R;
8384
}

0 commit comments

Comments
 (0)