bjoernager/rust
bjoernager/rust
1
Fork 0
Code Issues Pull requests Activity

Auto merge of #34652 - jseyfried:fix_expansion_perf, r=nrc

Browse source 
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
This commit is contained in:
bors 2016-07-06 20:04:11 -07:00 • committed by GitHub
commit de78655bca
8 changed files with 72 additions and 53 deletions
Download patch file Download diff file Expand all files Collapse all files

8
src/libsyntax/ext/expand.rs
View file

@ -237,7 +237,7 @@ fn expand_mac_invoc<T>(mac: ast::Mac, ident: Option<Ident>, attrs: Vec<ast::Attr
},
});
let marked_tts = mark_tts(tts, mark);
let marked_tts = mark_tts(&tts, mark);
Some(expandfun.expand(fld.cx, call_site, &marked_tts))
}
@ -257,7 +257,7 @@ fn expand_mac_invoc<T>(mac: ast::Mac, ident: Option<Ident>, attrs: Vec<ast::Attr
}
});
let marked_tts = mark_tts(tts, mark);
let marked_tts = mark_tts(&tts, mark);
Some(expander.expand(fld.cx, call_site, ident, marked_tts))
}
@ -1130,7 +1130,7 @@ impl Folder for Marker {
Spanned {
node: Mac_ {
path: self.fold_path(node.path),
tts: self.fold_tts(node.tts),
tts: self.fold_tts(&node.tts),
},
span: self.new_span(span),
}
@ -1145,7 +1145,7 @@ impl Folder for Marker {
}
// apply a given mark to the given token trees. Used prior to expansion of a macro.
fn mark_tts(tts: Vec<TokenTree>, m: Mrk) -> Vec<TokenTree> {
fn mark_tts(tts: &[TokenTree], m: Mrk) -> Vec<TokenTree> {
noop_fold_tts(tts, &mut Marker{mark:m, expn_id: None})
}

16
src/libsyntax/ext/quote.rs
View file

@ -32,6 +32,7 @@ pub mod rt {
use ext::base::ExtCtxt;
use parse::{self, token, classify};
use ptr::P;
use std::rc::Rc;
use tokenstream::{self, TokenTree};
@ -215,12 +216,12 @@ pub mod rt {
if self.node.style == ast::AttrStyle::Inner {
r.push(TokenTree::Token(self.span, token::Not));
}
r.push(TokenTree::Delimited(self.span, tokenstream::Delimited {
r.push(TokenTree::Delimited(self.span, Rc::new(tokenstream::Delimited {
delim: token::Bracket,
open_span: self.span,
tts: self.node.value.to_tokens(cx),
close_span: self.span,
}));
})));
r
}
}
@ -235,12 +236,12 @@ pub mod rt {
impl ToTokens for () {
fn to_tokens(&self, _cx: &ExtCtxt) -> Vec<TokenTree> {
vec![TokenTree::Delimited(DUMMY_SP, tokenstream::Delimited {
vec![TokenTree::Delimited(DUMMY_SP, Rc::new(tokenstream::Delimited {
delim: token::Paren,
open_span: DUMMY_SP,
tts: vec![],
close_span: DUMMY_SP,
})]
}))]
}
}
@ -791,9 +792,14 @@ fn statements_mk_tt(cx: &ExtCtxt, tt: &TokenTree, matcher: bool) -> Vec<ast::Stm
id_ext("tokenstream"),
id_ext("SequenceRepetition")];
let e_seq_struct = cx.expr_struct(sp, cx.path_global(sp, seq_path), fields);
let e_rc_new = cx.expr_call_global(sp, vec![id_ext("std"),
id_ext("rc"),
id_ext("Rc"),
id_ext("new")],
vec![e_seq_struct]);
let e_tok = cx.expr_call(sp,
mk_tt_path(cx, sp, "Sequence"),
vec!(e_sp, e_seq_struct));
vec!(e_sp, e_rc_new));
let e_push =
cx.expr_method_call(sp,
cx.expr_ident(sp, id_ext("tt")),

9
src/libsyntax/ext/tt/macro_rules.rs
View file

@ -28,6 +28,7 @@ use util::small_vector::SmallVector;
use std::cell::RefCell;
use std::collections::{HashMap};
use std::collections::hash_map::{Entry};
use std::rc::Rc;
struct ParserAnyMacro<'a> {
parser: RefCell<Parser<'a>>,
@ -262,7 +263,7 @@ pub fn compile<'cx>(cx: &'cx mut ExtCtxt,
let match_lhs_tok = MatchNt(lhs_nm, token::str_to_ident("tt"));
let match_rhs_tok = MatchNt(rhs_nm, token::str_to_ident("tt"));
let argument_gram = vec![
TokenTree::Sequence(DUMMY_SP, tokenstream::SequenceRepetition {
TokenTree::Sequence(DUMMY_SP, Rc::new(tokenstream::SequenceRepetition {
tts: vec![
TokenTree::Token(DUMMY_SP, match_lhs_tok),
TokenTree::Token(DUMMY_SP, token::FatArrow),
@ -271,14 +272,14 @@ pub fn compile<'cx>(cx: &'cx mut ExtCtxt,
separator: Some(token::Semi),
op: tokenstream::KleeneOp::OneOrMore,
num_captures: 2,
}),
})),
// to phase into semicolon-termination instead of semicolon-separation
TokenTree::Sequence(DUMMY_SP, tokenstream::SequenceRepetition {
TokenTree::Sequence(DUMMY_SP, Rc::new(tokenstream::SequenceRepetition {
tts: vec![TokenTree::Token(DUMMY_SP, token::Semi)],
separator: None,
op: tokenstream::KleeneOp::ZeroOrMore,
num_captures: 0
}),
})),
];
// Parse the macro_rules! invocation (`none` is for no interpolations):

4
src/libsyntax/ext/tt/transcribe.rs
View file

@ -79,11 +79,11 @@ pub fn new_tt_reader_with_doc_flag(sp_diag: &Handler,
let mut r = TtReader {
sp_diag: sp_diag,
stack: vec!(TtFrame {
forest: TokenTree::Sequence(DUMMY_SP, tokenstream::SequenceRepetition {
forest: TokenTree::Sequence(DUMMY_SP, Rc::new(tokenstream::SequenceRepetition {
tts: src,
// doesn't matter. This merely holds the root unzipping.
separator: None, op: tokenstream::KleeneOp::ZeroOrMore, num_captures: 0
}),
})),
idx: 0,
dotdotdoted: false,
sep: None,

38
src/libsyntax/fold.rs
View file

@ -28,6 +28,8 @@ use tokenstream::*;
use util::small_vector::SmallVector;
use util::move_map::MoveMap;
use std::rc::Rc;
pub trait Folder : Sized {
// Any additions to this trait should happen in form
// of a call to a public `noop_*` function that only calls
@ -222,11 +224,11 @@ pub trait Folder : Sized {
noop_fold_ty_params(tps, self)
}
fn fold_tt(&mut self, tt: TokenTree) -> TokenTree {
fn fold_tt(&mut self, tt: &TokenTree) -> TokenTree {
noop_fold_tt(tt, self)
}
fn fold_tts(&mut self, tts: Vec<TokenTree>) -> Vec<TokenTree> {
fn fold_tts(&mut self, tts: &[TokenTree]) -> Vec<TokenTree> {
noop_fold_tts(tts, self)
}
@ -501,7 +503,7 @@ pub fn noop_fold_mac<T: Folder>(Spanned {node, span}: Mac, fld: &mut T) -> Mac {
Spanned {
node: Mac_ {
path: fld.fold_path(node.path),
tts: fld.fold_tts(node.tts),
tts: fld.fold_tts(&node.tts),
},
span: fld.new_span(span)
}
@ -528,26 +530,32 @@ pub fn noop_fold_arg<T: Folder>(Arg {id, pat, ty}: Arg, fld: &mut T) -> Arg {
}
}
pub fn noop_fold_tt<T: Folder>(tt: TokenTree, fld: &mut T) -> TokenTree {
match tt {
pub fn noop_fold_tt<T: Folder>(tt: &TokenTree, fld: &mut T) -> TokenTree {
match *tt {
TokenTree::Token(span, ref tok) =>
TokenTree::Token(span, fld.fold_token(tok.clone())),
TokenTree::Delimited(span, delimed) => TokenTree::Delimited(span, Delimited {
TokenTree::Delimited(span, ref delimed) => {
TokenTree::Delimited(span, Rc::new(
Delimited {
delim: delimed.delim,
open_span: delimed.open_span,
tts: fld.fold_tts(delimed.tts),
tts: fld.fold_tts(&delimed.tts),
close_span: delimed.close_span,
}),
TokenTree::Sequence(span, seq) => TokenTree::Sequence(span, SequenceRepetition {
tts: fld.fold_tts(seq.tts),
}
))
},
TokenTree::Sequence(span, ref seq) =>
TokenTree::Sequence(span,
Rc::new(SequenceRepetition {
tts: fld.fold_tts(&seq.tts),
separator: seq.separator.clone().map(|tok| fld.fold_token(tok)),
..seq
}),
..**seq
})),
}
}
pub fn noop_fold_tts<T: Folder>(tts: Vec<TokenTree>, fld: &mut T) -> Vec<TokenTree> {
tts.move_map(|tt| fld.fold_tt(tt))
pub fn noop_fold_tts<T: Folder>(tts: &[TokenTree], fld: &mut T) -> Vec<TokenTree> {
tts.iter().map(|tt| fld.fold_tt(tt)).collect()
}
// 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)
token::NtIdent(Box::new(Spanned::<Ident>{node: fld.fold_ident(id.node), ..*id})),
token::NtMeta(meta_item) => token::NtMeta(fld.fold_meta_item(meta_item)),
token::NtPath(path) => token::NtPath(Box::new(fld.fold_path(*path))),
token::NtTT(tt) => token::NtTT(tt.map(|tt| fld.fold_tt(tt))),
token::NtTT(tt) => token::NtTT(P(fld.fold_tt(&tt))),
token::NtArm(arm) => token::NtArm(fld.fold_arm(arm)),
token::NtImplItem(arm) =>
token::NtImplItem(arm.map(|arm| fld.fold_impl_item(arm)

15
src/libsyntax/parse/mod.rs
View file

@ -662,6 +662,7 @@ pub fn integer_lit(s: &str,
#[cfg(test)]
mod tests {
use super::*;
use std::rc::Rc;
use syntax_pos::{Span, BytePos, Pos, NO_EXPANSION};
use codemap::Spanned;
use ast::{self, PatKind};
@ -763,7 +764,7 @@ mod tests {
)
if first_delimed.delim == token::Paren
&& ident.name.as_str() == "a" => {},
_ => panic!("value 3: {:?}", *first_delimed),
_ => panic!("value 3: {:?}", **first_delimed),
}
let tts = &second_delimed.tts[..];
match (tts.len(), tts.get(0), tts.get(1)) {
@ -774,10 +775,10 @@ mod tests {
)
if second_delimed.delim == token::Paren
&& ident.name.as_str() == "a" => {},
_ => panic!("value 4: {:?}", *second_delimed),
_ => panic!("value 4: {:?}", **second_delimed),
}
},
_ => panic!("value 2: {:?}", *macro_delimed),
_ => panic!("value 2: {:?}", **macro_delimed),
}
},
_ => panic!("value: {:?}",tts),
@ -793,7 +794,7 @@ mod tests {
TokenTree::Token(sp(3, 4), token::Ident(str_to_ident("a"))),
TokenTree::Delimited(
sp(5, 14),
tokenstream::Delimited {
Rc::new(tokenstream::Delimited {
delim: token::DelimToken::Paren,
open_span: sp(5, 6),
tts: vec![
@ -802,10 +803,10 @@ mod tests {
TokenTree::Token(sp(10, 13), token::Ident(str_to_ident("i32"))),
],
close_span: sp(13, 14),
}),
})),
TokenTree::Delimited(
sp(15, 21),
tokenstream::Delimited {
Rc::new(tokenstream::Delimited {
delim: token::DelimToken::Brace,
open_span: sp(15, 16),
tts: vec![
@ -813,7 +814,7 @@ mod tests {
TokenTree::Token(sp(18, 19), token::Semi),
],
close_span: sp(20, 21),
})
}))
];
assert_eq!(tts, expected);

9
src/libsyntax/parse/parser.rs
View file

@ -2688,12 +2688,13 @@ impl<'a> Parser<'a> {
)?;
let (sep, repeat) = self.parse_sep_and_kleene_op()?;
let name_num = macro_parser::count_names(&seq);
return Ok(TokenTree::Sequence(mk_sp(sp.lo, seq_span.hi), SequenceRepetition {
return Ok(TokenTree::Sequence(mk_sp(sp.lo, seq_span.hi),
Rc::new(SequenceRepetition {
tts: seq,
separator: sep,
op: repeat,
num_captures: name_num
}));
})));
} else if self.token.is_keyword(keywords::Crate) {
self.bump();
return Ok(TokenTree::Token(sp, SpecialVarNt(SpecialMacroVar::CrateMacroVar)));
@ -2848,12 +2849,12 @@ impl<'a> Parser<'a> {
_ => {}
}
Ok(TokenTree::Delimited(span, Delimited {
Ok(TokenTree::Delimited(span, Rc::new(Delimited {
delim: delim,
open_span: open_span,
tts: tts,
close_span: close_span,
}))
})))
},
_ => {
// invariants: the current token is not a left-delimiter,

10
src/libsyntax/tokenstream.rs
View file

@ -21,6 +21,8 @@ use parse::lexer::comments::{doc_comment_style, strip_doc_comment_decoration};
use parse::lexer;
use parse::token;
use std::rc::Rc;
/// A delimited sequence of token trees
#[derive(Clone, PartialEq, Eq, RustcEncodable, RustcDecodable, Hash, Debug)]
pub struct Delimited {
@ -94,13 +96,13 @@ pub enum TokenTree {
/// A single token
Token(Span, token::Token),
/// A delimited sequence of token trees
Delimited(Span, Delimited),
Delimited(Span, Rc<Delimited>),
// This only makes sense in MBE macros.
/// A kleene-style repetition sequence with a span
// FIXME(eddyb) #12938 Use DST.
Sequence(Span, SequenceRepetition),
Sequence(Span, Rc<SequenceRepetition>),
}
impl TokenTree {
@ -149,7 +151,7 @@ impl TokenTree {
Some(*cnt)
}).max().unwrap_or(0);
TokenTree::Delimited(sp, Delimited {
TokenTree::Delimited(sp, Rc::new(Delimited {
delim: token::Bracket,
open_span: sp,
tts: vec![TokenTree::Token(sp, token::Ident(token::str_to_ident("doc"))),
@ -157,7 +159,7 @@ impl TokenTree {
TokenTree::Token(sp, token::Literal(
token::StrRaw(token::intern(&stripped), num_of_hashes), None))],
close_span: sp,
})
}))
}
(&TokenTree::Delimited(_, ref delimed), _) => {
if index == 0 {