Skip to content

Commit cedb787

Browse files
committed
Remove MatcherPosHandle.
This type was a small performance win for `html5ever`, which uses a macro with hundreds of very simple rules that don't contain any metavariables. But this type is complicated (extra lifetimes) and perf-neutral for macros that do have metavariables. This commit removes `MatcherPosHandle`, simplifying things a lot. This increases the allocation rate for `html5ever` and similar cases a bit, but makes things easier for follow-up changes that will improve performance more than what we lost here.
1 parent 10644e0 commit cedb787

File tree

2 files changed

+23
-88
lines changed

2 files changed

+23
-88
lines changed

compiler/rustc_expand/src/lib.rs

+1
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
#![feature(associated_type_bounds)]
22
#![feature(associated_type_defaults)]
3+
#![feature(box_syntax)]
34
#![feature(crate_visibility_modifier)]
45
#![feature(decl_macro)]
56
#![feature(if_let_guard)]

compiler/rustc_expand/src/mbe/macro_parser.rs

+22-88
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,6 @@ use rustc_span::symbol::Ident;
8989
use std::borrow::Cow;
9090
use std::collections::hash_map::Entry::{Occupied, Vacant};
9191
use std::mem;
92-
use std::ops::{Deref, DerefMut};
9392

9493
// To avoid costly uniqueness checks, we require that `MatchSeq` always has a nonempty body.
9594

@@ -136,24 +135,8 @@ type NamedMatchVec = SmallVec<[NamedMatch; 4]>;
136135

137136
/// Represents a single "position" (aka "matcher position", aka "item"), as
138137
/// described in the module documentation.
139-
///
140-
/// Here:
141-
///
142-
/// - `'root` represents the lifetime of the stack slot that holds the root
143-
/// `MatcherPos`. As described in `MatcherPosHandle`, the root `MatcherPos`
144-
/// structure is stored on the stack, but subsequent instances are put into
145-
/// the heap.
146-
/// - `'tt` represents the lifetime of the token trees that this matcher
147-
/// position refers to.
148-
///
149-
/// It is important to distinguish these two lifetimes because we have a
150-
/// `SmallVec<TokenTreeOrTokenTreeSlice<'tt>>` below, and the destructor of
151-
/// that is considered to possibly access the data from its elements (it lacks
152-
/// a `#[may_dangle]` attribute). As a result, the compiler needs to know that
153-
/// all the elements in that `SmallVec` strictly outlive the root stack slot
154-
/// lifetime. By separating `'tt` from `'root`, we can show that.
155138
#[derive(Clone)]
156-
struct MatcherPos<'root, 'tt> {
139+
struct MatcherPos<'tt> {
157140
/// The token or slice of tokens that make up the matcher. `elts` is short for "elements".
158141
top_elts: TokenTreeOrTokenTreeSlice<'tt>,
159142

@@ -185,7 +168,7 @@ struct MatcherPos<'root, 'tt> {
185168
match_hi: usize,
186169

187170
/// This field is only used if we are matching a repetition.
188-
repetition: Option<MatcherPosRepetition<'root, 'tt>>,
171+
repetition: Option<MatcherPosRepetition<'tt>>,
189172

190173
/// Specifically used to "unzip" token trees. By "unzip", we mean to unwrap the delimiters from
191174
/// a delimited token tree (e.g., something wrapped in `(` `)`) or to get the contents of a doc
@@ -200,9 +183,9 @@ struct MatcherPos<'root, 'tt> {
200183

201184
// This type is used a lot. Make sure it doesn't unintentionally get bigger.
202185
#[cfg(all(target_arch = "x86_64", target_pointer_width = "64"))]
203-
rustc_data_structures::static_assert_size!(MatcherPos<'_, '_>, 240);
186+
rustc_data_structures::static_assert_size!(MatcherPos<'_>, 232);
204187

205-
impl<'root, 'tt> MatcherPos<'root, 'tt> {
188+
impl<'tt> MatcherPos<'tt> {
206189
/// `len` `Vec`s (initially shared and empty) that will store matches of metavars.
207190
fn create_matches(len: usize) -> Box<[Lrc<NamedMatchVec>]> {
208191
if len == 0 {
@@ -241,11 +224,7 @@ impl<'root, 'tt> MatcherPos<'root, 'tt> {
241224
}
242225
}
243226

244-
fn repetition(
245-
up: MatcherPosHandle<'root, 'tt>,
246-
sp: DelimSpan,
247-
seq: Lrc<SequenceRepetition>,
248-
) -> Self {
227+
fn repetition(up: Box<MatcherPos<'tt>>, sp: DelimSpan, seq: Lrc<SequenceRepetition>) -> Self {
249228
MatcherPos {
250229
stack: smallvec![],
251230
idx: 0,
@@ -270,7 +249,7 @@ impl<'root, 'tt> MatcherPos<'root, 'tt> {
270249
}
271250

272251
#[derive(Clone)]
273-
struct MatcherPosRepetition<'root, 'tt> {
252+
struct MatcherPosRepetition<'tt> {
274253
/// The KleeneOp of this sequence.
275254
seq_op: mbe::KleeneOp,
276255

@@ -279,55 +258,12 @@ struct MatcherPosRepetition<'root, 'tt> {
279258

280259
/// The "parent" matcher position. That is, the matcher position just before we enter the
281260
/// sequence.
282-
up: MatcherPosHandle<'root, 'tt>,
283-
}
284-
285-
// Lots of MatcherPos instances are created at runtime. Allocating them on the
286-
// heap is slow. Furthermore, using SmallVec<MatcherPos> to allocate them all
287-
// on the stack is also slow, because MatcherPos is quite a large type and
288-
// instances get moved around a lot between vectors, which requires lots of
289-
// slow memcpy calls.
290-
//
291-
// Therefore, the initial MatcherPos is always allocated on the stack,
292-
// subsequent ones (of which there aren't that many) are allocated on the heap,
293-
// and this type is used to encapsulate both cases.
294-
enum MatcherPosHandle<'root, 'tt> {
295-
Ref(&'root mut MatcherPos<'root, 'tt>),
296-
Box(Box<MatcherPos<'root, 'tt>>),
261+
up: Box<MatcherPos<'tt>>,
297262
}
298263

299-
impl<'root, 'tt> Clone for MatcherPosHandle<'root, 'tt> {
300-
// This always produces a new Box.
301-
fn clone(&self) -> Self {
302-
MatcherPosHandle::Box(match *self {
303-
MatcherPosHandle::Ref(ref r) => Box::new((**r).clone()),
304-
MatcherPosHandle::Box(ref b) => b.clone(),
305-
})
306-
}
307-
}
308-
309-
impl<'root, 'tt> Deref for MatcherPosHandle<'root, 'tt> {
310-
type Target = MatcherPos<'root, 'tt>;
311-
fn deref(&self) -> &Self::Target {
312-
match *self {
313-
MatcherPosHandle::Ref(ref r) => r,
314-
MatcherPosHandle::Box(ref b) => b,
315-
}
316-
}
317-
}
318-
319-
impl<'root, 'tt> DerefMut for MatcherPosHandle<'root, 'tt> {
320-
fn deref_mut(&mut self) -> &mut MatcherPos<'root, 'tt> {
321-
match *self {
322-
MatcherPosHandle::Ref(ref mut r) => r,
323-
MatcherPosHandle::Box(ref mut b) => b,
324-
}
325-
}
326-
}
327-
328-
enum EofItems<'root, 'tt> {
264+
enum EofItems<'tt> {
329265
None,
330-
One(MatcherPosHandle<'root, 'tt>),
266+
One(Box<MatcherPos<'tt>>),
331267
Multiple,
332268
}
333269

@@ -494,6 +430,10 @@ fn token_name_eq(t1: &Token, t2: &Token) -> bool {
494430

495431
pub struct TtParser {
496432
macro_name: Ident,
433+
434+
cur_items: Vec<Box<MatcherPos<'tt>>>,
435+
next_items: Vec<Box<MatcherPos<'tt>>>,
436+
bb_items: Vec<Box<MatcherPos<'tt>>>,
497437
}
498438

499439
impl TtParser {
@@ -520,13 +460,13 @@ impl TtParser {
520460
///
521461
/// `Some(result)` if everything is finished, `None` otherwise. Note that matches are kept
522462
/// track of through the items generated.
523-
fn parse_tt_inner<'root, 'tt>(
463+
fn parse_tt_inner<'tt>(
524464
&self,
525465
sess: &ParseSess,
526466
ms: &[TokenTree],
527-
cur_items: &mut SmallVec<[MatcherPosHandle<'root, 'tt>; 1]>,
528-
next_items: &mut SmallVec<[MatcherPosHandle<'root, 'tt>; 1]>,
529-
bb_items: &mut SmallVec<[MatcherPosHandle<'root, 'tt>; 1]>,
467+
cur_items: &mut SmallVec<[Box<MatcherPos<'tt>>; 1]>,
468+
next_items: &mut SmallVec<[Box<MatcherPos<'tt>>; 1]>,
469+
bb_items: &mut SmallVec<[Box<MatcherPos<'tt>>; 1]>,
530470
token: &Token,
531471
) -> Option<NamedParseResult> {
532472
// Matcher positions that would be valid if the macro invocation was over now. Only
@@ -570,9 +510,7 @@ impl TtParser {
570510
}
571511

572512
// Allow for the possibility of one or more matches of this sequence.
573-
cur_items.push(MatcherPosHandle::Box(Box::new(MatcherPos::repetition(
574-
item, sp, seq,
575-
))));
513+
cur_items.push(box MatcherPos::repetition(item, sp, seq));
576514
}
577515

578516
TokenTree::MetaVarDecl(span, _, None) => {
@@ -706,11 +644,7 @@ impl TtParser {
706644
// `parse_tt_inner` then processes all of these possible matcher positions and produces
707645
// possible next positions into `next_items`. After some post-processing, the contents of
708646
// `next_items` replenish `cur_items` and we start over again.
709-
//
710-
// This MatcherPos instance is allocated on the stack. All others -- and there are
711-
// frequently *no* others! -- are allocated on the heap.
712-
let mut initial = MatcherPos::new(ms);
713-
let mut cur_items = smallvec![MatcherPosHandle::Ref(&mut initial)];
647+
let mut cur_items = smallvec![box MatcherPos::new(ms)];
714648

715649
loop {
716650
let mut next_items = SmallVec::new();
@@ -793,10 +727,10 @@ impl TtParser {
793727
}
794728
}
795729

796-
fn ambiguity_error<'root, 'tt>(
730+
fn ambiguity_error<'tt>(
797731
&self,
798-
next_items: SmallVec<[MatcherPosHandle<'root, 'tt>; 1]>,
799-
bb_items: SmallVec<[MatcherPosHandle<'root, 'tt>; 1]>,
732+
next_items: SmallVec<[Box<MatcherPos<'tt>>; 1]>,
733+
bb_items: SmallVec<[Box<MatcherPos<'tt>>; 1]>,
800734
token_span: rustc_span::Span,
801735
) -> NamedParseResult {
802736
let nts = bb_items

0 commit comments

Comments
 (0)