Skip to content

Commit de78655

Browse files
borsbors
authored
Auto merge of #34652 - jseyfried:fix_expansion_perf, r=nrc
Fix expansion performance regression **syntax-[breaking-change] cc #31645** This fixes #34630 by reverting commit 5bf7970 of PR #33943, which landed in #34424. By removing the `Rc<_>` wrapping around `Delimited` and `SequenceRepetition` in `TokenTree`, 5bf7970 made cloning `TokenTree`s more expensive. While this had no measurable performance impact on the compiler's crates, it caused an order of magnitude performance regression on some macro-heavy code in the wild. I believe this is due to clones of `TokenTree`s in `macro_parser.rs` and/or `macro_rules.rs`. r? @nrc
2 parents 5c674a1 + 547a930 commit de78655

File tree

8 files changed

+72
-53
lines changed

8 files changed

+72
-53
lines changed

‎src/libsyntax/ext/expand.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -237,7 +237,7 @@ fn expand_mac_invoc<T>(mac: ast::Mac, ident: Option<Ident>, attrs: Vec<ast::Attr
237237
},
238238
});
239239

240-
let marked_tts = mark_tts(tts, mark);
240+
let marked_tts = mark_tts(&tts, mark);
241241
Some(expandfun.expand(fld.cx, call_site, &marked_tts))
242242
}
243243

@@ -257,7 +257,7 @@ fn expand_mac_invoc<T>(mac: ast::Mac, ident: Option<Ident>, attrs: Vec<ast::Attr
257257
}
258258
});
259259

260-
let marked_tts = mark_tts(tts, mark);
260+
let marked_tts = mark_tts(&tts, mark);
261261
Some(expander.expand(fld.cx, call_site, ident, marked_tts))
262262
}
263263

@@ -1130,7 +1130,7 @@ impl Folder for Marker {
11301130
Spanned {
11311131
node: Mac_ {
11321132
path: self.fold_path(node.path),
1133-
tts: self.fold_tts(node.tts),
1133+
tts: self.fold_tts(&node.tts),
11341134
},
11351135
span: self.new_span(span),
11361136
}
@@ -1145,7 +1145,7 @@ impl Folder for Marker {
11451145
}
11461146

11471147
// apply a given mark to the given token trees. Used prior to expansion of a macro.
1148-
fn mark_tts(tts: Vec<TokenTree>, m: Mrk) -> Vec<TokenTree> {
1148+
fn mark_tts(tts: &[TokenTree], m: Mrk) -> Vec<TokenTree> {
11491149
noop_fold_tts(tts, &mut Marker{mark:m, expn_id: None})
11501150
}
11511151

‎src/libsyntax/ext/quote.rs

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ pub mod rt {
3232
use ext::base::ExtCtxt;
3333
use parse::{self, token, classify};
3434
use ptr::P;
35+
use std::rc::Rc;
3536

3637
use tokenstream::{self, TokenTree};
3738

@@ -215,12 +216,12 @@ pub mod rt {
215216
if self.node.style == ast::AttrStyle::Inner {
216217
r.push(TokenTree::Token(self.span, token::Not));
217218
}
218-
r.push(TokenTree::Delimited(self.span, tokenstream::Delimited {
219+
r.push(TokenTree::Delimited(self.span, Rc::new(tokenstream::Delimited {
219220
delim: token::Bracket,
220221
open_span: self.span,
221222
tts: self.node.value.to_tokens(cx),
222223
close_span: self.span,
223-
}));
224+
})));
224225
r
225226
}
226227
}
@@ -235,12 +236,12 @@ pub mod rt {
235236

236237
impl ToTokens for () {
237238
fn to_tokens(&self, _cx: &ExtCtxt) -> Vec<TokenTree> {
238-
vec![TokenTree::Delimited(DUMMY_SP, tokenstream::Delimited {
239+
vec![TokenTree::Delimited(DUMMY_SP, Rc::new(tokenstream::Delimited {
239240
delim: token::Paren,
240241
open_span: DUMMY_SP,
241242
tts: vec![],
242243
close_span: DUMMY_SP,
243-
})]
244+
}))]
244245
}
245246
}
246247

@@ -791,9 +792,14 @@ fn statements_mk_tt(cx: &ExtCtxt, tt: &TokenTree, matcher: bool) -> Vec<ast::Stm
791792
id_ext("tokenstream"),
792793
id_ext("SequenceRepetition")];
793794
let e_seq_struct = cx.expr_struct(sp, cx.path_global(sp, seq_path), fields);
795+
let e_rc_new = cx.expr_call_global(sp, vec![id_ext("std"),
796+
id_ext("rc"),
797+
id_ext("Rc"),
798+
id_ext("new")],
799+
vec![e_seq_struct]);
794800
let e_tok = cx.expr_call(sp,
795801
mk_tt_path(cx, sp, "Sequence"),
796-
vec!(e_sp, e_seq_struct));
802+
vec!(e_sp, e_rc_new));
797803
let e_push =
798804
cx.expr_method_call(sp,
799805
cx.expr_ident(sp, id_ext("tt")),

‎src/libsyntax/ext/tt/macro_rules.rs

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ use util::small_vector::SmallVector;
2828
use std::cell::RefCell;
2929
use std::collections::{HashMap};
3030
use std::collections::hash_map::{Entry};
31+
use std::rc::Rc;
3132

3233
struct ParserAnyMacro<'a> {
3334
parser: RefCell<Parser<'a>>,
@@ -262,7 +263,7 @@ pub fn compile<'cx>(cx: &'cx mut ExtCtxt,
262263
let match_lhs_tok = MatchNt(lhs_nm, token::str_to_ident("tt"));
263264
let match_rhs_tok = MatchNt(rhs_nm, token::str_to_ident("tt"));
264265
let argument_gram = vec![
265-
TokenTree::Sequence(DUMMY_SP, tokenstream::SequenceRepetition {
266+
TokenTree::Sequence(DUMMY_SP, Rc::new(tokenstream::SequenceRepetition {
266267
tts: vec![
267268
TokenTree::Token(DUMMY_SP, match_lhs_tok),
268269
TokenTree::Token(DUMMY_SP, token::FatArrow),
@@ -271,14 +272,14 @@ pub fn compile<'cx>(cx: &'cx mut ExtCtxt,
271272
separator: Some(token::Semi),
272273
op: tokenstream::KleeneOp::OneOrMore,
273274
num_captures: 2,
274-
}),
275+
})),
275276
// to phase into semicolon-termination instead of semicolon-separation
276-
TokenTree::Sequence(DUMMY_SP, tokenstream::SequenceRepetition {
277+
TokenTree::Sequence(DUMMY_SP, Rc::new(tokenstream::SequenceRepetition {
277278
tts: vec![TokenTree::Token(DUMMY_SP, token::Semi)],
278279
separator: None,
279280
op: tokenstream::KleeneOp::ZeroOrMore,
280281
num_captures: 0
281-
}),
282+
})),
282283
];
283284

284285
// Parse the macro_rules! invocation (`none` is for no interpolations):

‎src/libsyntax/ext/tt/transcribe.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -79,11 +79,11 @@ pub fn new_tt_reader_with_doc_flag(sp_diag: &Handler,
7979
let mut r = TtReader {
8080
sp_diag: sp_diag,
8181
stack: vec!(TtFrame {
82-
forest: TokenTree::Sequence(DUMMY_SP, tokenstream::SequenceRepetition {
82+
forest: TokenTree::Sequence(DUMMY_SP, Rc::new(tokenstream::SequenceRepetition {
8383
tts: src,
8484
// doesn't matter. This merely holds the root unzipping.
8585
separator: None, op: tokenstream::KleeneOp::ZeroOrMore, num_captures: 0
86-
}),
86+
})),
8787
idx: 0,
8888
dotdotdoted: false,
8989
sep: None,

‎src/libsyntax/fold.rs

Lines changed: 27 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,8 @@ use tokenstream::*;
2828
use util::small_vector::SmallVector;
2929
use util::move_map::MoveMap;
3030

31+
use std::rc::Rc;
32+
3133
pub trait Folder : Sized {
3234
// Any additions to this trait should happen in form
3335
// of a call to a public `noop_*` function that only calls
@@ -222,11 +224,11 @@ pub trait Folder : Sized {
222224
noop_fold_ty_params(tps, self)
223225
}
224226

225-
fn fold_tt(&mut self, tt: TokenTree) -> TokenTree {
227+
fn fold_tt(&mut self, tt: &TokenTree) -> TokenTree {
226228
noop_fold_tt(tt, self)
227229
}
228230

229-
fn fold_tts(&mut self, tts: Vec<TokenTree>) -> Vec<TokenTree> {
231+
fn fold_tts(&mut self, tts: &[TokenTree]) -> Vec<TokenTree> {
230232
noop_fold_tts(tts, self)
231233
}
232234

@@ -501,7 +503,7 @@ pub fn noop_fold_mac<T: Folder>(Spanned {node, span}: Mac, fld: &mut T) -> Mac {
501503
Spanned {
502504
node: Mac_ {
503505
path: fld.fold_path(node.path),
504-
tts: fld.fold_tts(node.tts),
506+
tts: fld.fold_tts(&node.tts),
505507
},
506508
span: fld.new_span(span)
507509
}
@@ -528,26 +530,32 @@ pub fn noop_fold_arg<T: Folder>(Arg {id, pat, ty}: Arg, fld: &mut T) -> Arg {
528530
}
529531
}
530532

531-
pub fn noop_fold_tt<T: Folder>(tt: TokenTree, fld: &mut T) -> TokenTree {
532-
match tt {
533+
pub fn noop_fold_tt<T: Folder>(tt: &TokenTree, fld: &mut T) -> TokenTree {
534+
match *tt {
533535
TokenTree::Token(span, ref tok) =>
534536
TokenTree::Token(span, fld.fold_token(tok.clone())),
535-
TokenTree::Delimited(span, delimed) => TokenTree::Delimited(span, Delimited {
536-
delim: delimed.delim,
537-
open_span: delimed.open_span,
538-
tts: fld.fold_tts(delimed.tts),
539-
close_span: delimed.close_span,
540-
}),
541-
TokenTree::Sequence(span, seq) => TokenTree::Sequence(span, SequenceRepetition {
542-
tts: fld.fold_tts(seq.tts),
543-
separator: seq.separator.clone().map(|tok| fld.fold_token(tok)),
544-
..seq
545-
}),
537+
TokenTree::Delimited(span, ref delimed) => {
538+
TokenTree::Delimited(span, Rc::new(
539+
Delimited {
540+
delim: delimed.delim,
541+
open_span: delimed.open_span,
542+
tts: fld.fold_tts(&delimed.tts),
543+
close_span: delimed.close_span,
544+
}
545+
))
546+
},
547+
TokenTree::Sequence(span, ref seq) =>
548+
TokenTree::Sequence(span,
549+
Rc::new(SequenceRepetition {
550+
tts: fld.fold_tts(&seq.tts),
551+
separator: seq.separator.clone().map(|tok| fld.fold_token(tok)),
552+
..**seq
553+
})),
546554
}
547555
}
548556

549-
pub fn noop_fold_tts<T: Folder>(tts: Vec<TokenTree>, fld: &mut T) -> Vec<TokenTree> {
550-
tts.move_map(|tt| fld.fold_tt(tt))
557+
pub fn noop_fold_tts<T: Folder>(tts: &[TokenTree], fld: &mut T) -> Vec<TokenTree> {
558+
tts.iter().map(|tt| fld.fold_tt(tt)).collect()
551559
}
552560

553561
// apply ident folder if it's an ident, apply other folds to interpolated nodes
@@ -605,7 +613,7 @@ pub fn noop_fold_interpolated<T: Folder>(nt: token::Nonterminal, fld: &mut T)
605613
token::NtIdent(Box::new(Spanned::<Ident>{node: fld.fold_ident(id.node), ..*id})),
606614
token::NtMeta(meta_item) => token::NtMeta(fld.fold_meta_item(meta_item)),
607615
token::NtPath(path) => token::NtPath(Box::new(fld.fold_path(*path))),
608-
token::NtTT(tt) => token::NtTT(tt.map(|tt| fld.fold_tt(tt))),
616+
token::NtTT(tt) => token::NtTT(P(fld.fold_tt(&tt))),
609617
token::NtArm(arm) => token::NtArm(fld.fold_arm(arm)),
610618
token::NtImplItem(arm) =>
611619
token::NtImplItem(arm.map(|arm| fld.fold_impl_item(arm)

‎src/libsyntax/parse/mod.rs

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -662,6 +662,7 @@ pub fn integer_lit(s: &str,
662662
#[cfg(test)]
663663
mod tests {
664664
use super::*;
665+
use std::rc::Rc;
665666
use syntax_pos::{Span, BytePos, Pos, NO_EXPANSION};
666667
use codemap::Spanned;
667668
use ast::{self, PatKind};
@@ -763,7 +764,7 @@ mod tests {
763764
)
764765
if first_delimed.delim == token::Paren
765766
&& ident.name.as_str() == "a" => {},
766-
_ => panic!("value 3: {:?}", *first_delimed),
767+
_ => panic!("value 3: {:?}", **first_delimed),
767768
}
768769
let tts = &second_delimed.tts[..];
769770
match (tts.len(), tts.get(0), tts.get(1)) {
@@ -774,10 +775,10 @@ mod tests {
774775
)
775776
if second_delimed.delim == token::Paren
776777
&& ident.name.as_str() == "a" => {},
777-
_ => panic!("value 4: {:?}", *second_delimed),
778+
_ => panic!("value 4: {:?}", **second_delimed),
778779
}
779780
},
780-
_ => panic!("value 2: {:?}", *macro_delimed),
781+
_ => panic!("value 2: {:?}", **macro_delimed),
781782
}
782783
},
783784
_ => panic!("value: {:?}",tts),
@@ -793,7 +794,7 @@ mod tests {
793794
TokenTree::Token(sp(3, 4), token::Ident(str_to_ident("a"))),
794795
TokenTree::Delimited(
795796
sp(5, 14),
796-
tokenstream::Delimited {
797+
Rc::new(tokenstream::Delimited {
797798
delim: token::DelimToken::Paren,
798799
open_span: sp(5, 6),
799800
tts: vec![
@@ -802,18 +803,18 @@ mod tests {
802803
TokenTree::Token(sp(10, 13), token::Ident(str_to_ident("i32"))),
803804
],
804805
close_span: sp(13, 14),
805-
}),
806+
})),
806807
TokenTree::Delimited(
807808
sp(15, 21),
808-
tokenstream::Delimited {
809+
Rc::new(tokenstream::Delimited {
809810
delim: token::DelimToken::Brace,
810811
open_span: sp(15, 16),
811812
tts: vec![
812813
TokenTree::Token(sp(17, 18), token::Ident(str_to_ident("b"))),
813814
TokenTree::Token(sp(18, 19), token::Semi),
814815
],
815816
close_span: sp(20, 21),
816-
})
817+
}))
817818
];
818819

819820
assert_eq!(tts, expected);

‎src/libsyntax/parse/parser.rs

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2688,12 +2688,13 @@ impl<'a> Parser<'a> {
26882688
)?;
26892689
let (sep, repeat) = self.parse_sep_and_kleene_op()?;
26902690
let name_num = macro_parser::count_names(&seq);
2691-
return Ok(TokenTree::Sequence(mk_sp(sp.lo, seq_span.hi), SequenceRepetition {
2692-
tts: seq,
2693-
separator: sep,
2694-
op: repeat,
2695-
num_captures: name_num
2696-
}));
2691+
return Ok(TokenTree::Sequence(mk_sp(sp.lo, seq_span.hi),
2692+
Rc::new(SequenceRepetition {
2693+
tts: seq,
2694+
separator: sep,
2695+
op: repeat,
2696+
num_captures: name_num
2697+
})));
26972698
} else if self.token.is_keyword(keywords::Crate) {
26982699
self.bump();
26992700
return Ok(TokenTree::Token(sp, SpecialVarNt(SpecialMacroVar::CrateMacroVar)));
@@ -2848,12 +2849,12 @@ impl<'a> Parser<'a> {
28482849
_ => {}
28492850
}
28502851

2851-
Ok(TokenTree::Delimited(span, Delimited {
2852+
Ok(TokenTree::Delimited(span, Rc::new(Delimited {
28522853
delim: delim,
28532854
open_span: open_span,
28542855
tts: tts,
28552856
close_span: close_span,
2856-
}))
2857+
})))
28572858
},
28582859
_ => {
28592860
// invariants: the current token is not a left-delimiter,

‎src/libsyntax/tokenstream.rs

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,8 @@ use parse::lexer::comments::{doc_comment_style, strip_doc_comment_decoration};
2121
use parse::lexer;
2222
use parse::token;
2323

24+
use std::rc::Rc;
25+
2426
/// A delimited sequence of token trees
2527
#[derive(Clone, PartialEq, Eq, RustcEncodable, RustcDecodable, Hash, Debug)]
2628
pub struct Delimited {
@@ -94,13 +96,13 @@ pub enum TokenTree {
9496
/// A single token
9597
Token(Span, token::Token),
9698
/// A delimited sequence of token trees
97-
Delimited(Span, Delimited),
99+
Delimited(Span, Rc<Delimited>),
98100

99101
// This only makes sense in MBE macros.
100102

101103
/// A kleene-style repetition sequence with a span
102104
// FIXME(eddyb) #12938 Use DST.
103-
Sequence(Span, SequenceRepetition),
105+
Sequence(Span, Rc<SequenceRepetition>),
104106
}
105107

106108
impl TokenTree {
@@ -149,15 +151,15 @@ impl TokenTree {
149151
Some(*cnt)
150152
}).max().unwrap_or(0);
151153

152-
TokenTree::Delimited(sp, Delimited {
154+
TokenTree::Delimited(sp, Rc::new(Delimited {
153155
delim: token::Bracket,
154156
open_span: sp,
155157
tts: vec![TokenTree::Token(sp, token::Ident(token::str_to_ident("doc"))),
156158
TokenTree::Token(sp, token::Eq),
157159
TokenTree::Token(sp, token::Literal(
158160
token::StrRaw(token::intern(&stripped), num_of_hashes), None))],
159161
close_span: sp,
160-
})
162+
}))
161163
}
162164
(&TokenTree::Delimited(_, ref delimed), _) => {
163165
if index == 0 {

0 commit comments

Comments
 (0)