Skip to content
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

Avoid more allocations when compiling html5ever #37373

Merged
merged 3 commits into from
Oct 29, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 6 additions & 8 deletions src/libsyntax/ext/tt/macro_parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@ use parse::token;
use print::pprust;
use ptr::P;
use tokenstream::{self, TokenTree};
use util::small_vector::SmallVector;

use std::mem;
use std::rc::Rc;
Expand All @@ -104,7 +105,7 @@ use std::collections::hash_map::Entry::{Vacant, Occupied};
#[derive(Clone)]
enum TokenTreeOrTokenTreeVec {
Tt(tokenstream::TokenTree),
TtSeq(Rc<Vec<tokenstream::TokenTree>>),
TtSeq(Vec<tokenstream::TokenTree>),
}

impl TokenTreeOrTokenTreeVec {
Expand Down Expand Up @@ -161,7 +162,7 @@ pub fn count_names(ms: &[TokenTree]) -> usize {
})
}

pub fn initial_matcher_pos(ms: Rc<Vec<TokenTree>>, sep: Option<Token>, lo: BytePos)
pub fn initial_matcher_pos(ms: Vec<TokenTree>, sep: Option<Token>, lo: BytePos)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe this means that we are now copying/moving the Vec; perhaps we only need &[TokenTree] here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Vec gets put into the MatcherPos, so if it becomes a reference then MatcherPos will need a lifetime paramter and things get more complicated. Copying a Vec shouldn't be that expensive because only three words get copied, right? I did a Cachegrind run and this commit caused the number of instructions executed to drop.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cloning a Vec copies all the allocated elements. But it can probably use memcpy, so that should be fast.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was under the impression that passing a Vec as an argument to a function doesn't clone it. Have I got that wrong?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh right... forget I said anything, I thought the Rc was there to prevent the cloning.

-> Box<MatcherPos> {
let match_idx_hi = count_names(&ms[..]);
let matches: Vec<_> = (0..match_idx_hi).map(|_| Vec::new()).collect();
Expand Down Expand Up @@ -284,12 +285,9 @@ pub fn parse(sess: &ParseSess,
mut rdr: TtReader,
ms: &[TokenTree])
-> NamedParseResult {
let mut cur_eis = Vec::new();
cur_eis.push(initial_matcher_pos(Rc::new(ms.iter()
.cloned()
.collect()),
None,
rdr.peek().sp.lo));
let mut cur_eis = SmallVector::one(initial_matcher_pos(ms.to_owned(),
None,
rdr.peek().sp.lo));

loop {
let mut bb_eis = Vec::new(); // black-box parsed by parser.rs
Expand Down
7 changes: 4 additions & 3 deletions src/libsyntax/ext/tt/transcribe.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,15 @@
use self::LockstepIterSize::*;

use ast::Ident;
use syntax_pos::{Span, DUMMY_SP};
use errors::{Handler, DiagnosticBuilder};
use ext::tt::macro_parser::{NamedMatch, MatchedSeq, MatchedNonterminal};
use parse::token::{DocComment, MatchNt, SubstNt};
use parse::token::{Token, Interpolated, NtIdent, NtTT};
use parse::token;
use parse::lexer::TokenAndSpan;
use syntax_pos::{Span, DUMMY_SP};
use tokenstream::{self, TokenTree};
use util::small_vector::SmallVector;

use std::rc::Rc;
use std::ops::Add;
Expand All @@ -36,7 +37,7 @@ struct TtFrame {
pub struct TtReader<'a> {
pub sp_diag: &'a Handler,
/// the unzipped tree:
stack: Vec<TtFrame>,
stack: SmallVector<TtFrame>,
/* for MBE-style macro transcription */
interpolations: HashMap<Ident, Rc<NamedMatch>>,

Expand Down Expand Up @@ -74,7 +75,7 @@ pub fn new_tt_reader_with_doc_flag(sp_diag: &Handler,
-> TtReader {
let mut r = TtReader {
sp_diag: sp_diag,
stack: vec!(TtFrame {
stack: SmallVector::one(TtFrame {
forest: TokenTree::Sequence(DUMMY_SP, Rc::new(tokenstream::SequenceRepetition {
tts: src,
// doesn't matter. This merely holds the root unzipping.
Expand Down
1 change: 1 addition & 0 deletions src/libsyntax/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
#![feature(rustc_diagnostic_macros)]
#![feature(specialization)]

extern crate core;
extern crate serialize;
extern crate term;
extern crate libc;
Expand Down
50 changes: 40 additions & 10 deletions src/libsyntax/util/small_vector.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
use self::SmallVectorRepr::*;
use self::IntoIterRepr::*;

use core::ops;
use std::iter::{IntoIterator, FromIterator};
use std::mem;
use std::slice;
Expand All @@ -19,10 +20,12 @@ use std::vec;
use util::move_map::MoveMap;

/// A vector type optimized for cases where the size is almost always 0 or 1
#[derive(Clone)]
pub struct SmallVector<T> {
repr: SmallVectorRepr<T>,
}

#[derive(Clone)]
enum SmallVectorRepr<T> {
Zero,
One(T),
Expand Down Expand Up @@ -75,16 +78,11 @@ impl<T> SmallVector<T> {
}

pub fn as_slice(&self) -> &[T] {
match self.repr {
Zero => {
let result: &[T] = &[];
result
}
One(ref v) => {
unsafe { slice::from_raw_parts(v, 1) }
}
Many(ref vs) => vs
}
self
}

pub fn as_mut_slice(&mut self) -> &mut [T] {
self
}

pub fn pop(&mut self) -> Option<T> {
Expand Down Expand Up @@ -163,6 +161,38 @@ impl<T> SmallVector<T> {
}
}

impl<T> ops::Deref for SmallVector<T> {
type Target = [T];

fn deref(&self) -> &[T] {
match self.repr {
Zero => {
let result: &[T] = &[];
result
}
One(ref v) => {
unsafe { slice::from_raw_parts(v, 1) }
}
Many(ref vs) => vs
}
}
}

impl<T> ops::DerefMut for SmallVector<T> {
fn deref_mut(&mut self) -> &mut [T] {
match self.repr {
Zero => {
let result: &mut [T] = &mut [];
result
}
One(ref mut v) => {
unsafe { slice::from_raw_parts_mut(v, 1) }
}
Many(ref mut vs) => vs
}
}
}

impl<T> IntoIterator for SmallVector<T> {
type Item = T;
type IntoIter = IntoIter<T>;
Expand Down