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

Reduce size of syntax::tokenstream::TokenTree #54535

Closed
dtolnay opened this issue Sep 24, 2018 · 7 comments
Closed

Reduce size of syntax::tokenstream::TokenTree #54535

dtolnay opened this issue Sep 24, 2018 · 7 comments
Labels
I-compiletime Issue: Problems and improvements with respect to compile times.

Comments

@dtolnay
Copy link
Member

dtolnay commented Sep 24, 2018

#53902 raised the size of TokenTree from 32 bytes to 40 bytes, resulting in increased max-rss across a variety of benchmarks by up to 6%.

Here are the data structures involved: playground

Even among the 32 bytes a lot of those were padding. It should be possible to rearrange a few things to drop back to 32 bytes while retaining all the information currently represented.

@nnethercote

@dtolnay
Copy link
Member Author

dtolnay commented Sep 24, 2018

It may be unfortunate to do this but inlining all the fields of Delimited into TokenTree is able to pack things more efficiently and fit in 32 bytes.

  enum TokenTree {
      Token(Span, Token),
-     Delimited(DelimSpan, Delimited),
+     Delimited(DelimSpan, DelimToken, ThinTokenStream),
  }
-
- struct Delimited {
-     delim: DelimToken,
-     tts: ThinTokenStream,
- }

@eddyb
Copy link
Member

eddyb commented Sep 24, 2018

@dtolnay Were there APIs on Delimited? I think that's a reasonable inline.
Would it also be possible to move DelimSpan inside Delimited? Or would that not help?

@dtolnay
Copy link
Member Author

dtolnay commented Sep 24, 2018

Inlining DelimSpan does not help.

There are five methods on Delimited:

impl Delimited {
/// Returns the opening delimiter as a token.
pub fn open_token(&self) -> token::Token {
token::OpenDelim(self.delim)
}
/// Returns the closing delimiter as a token.
pub fn close_token(&self) -> token::Token {
token::CloseDelim(self.delim)
}
/// Returns the opening delimiter as a token tree.
pub fn open_tt(&self, span: Span) -> TokenTree {
let open_span = if span.is_dummy() {
span
} else {
span.with_hi(span.lo() + BytePos(self.delim.len() as u32))
};
TokenTree::Token(open_span, self.open_token())
}
/// Returns the closing delimiter as a token tree.
pub fn close_tt(&self, span: Span) -> TokenTree {
let close_span = if span.is_dummy() {
span
} else {
span.with_lo(span.hi() - BytePos(self.delim.len() as u32))
};
TokenTree::Token(close_span, self.close_token())
}
/// Returns the token trees inside the delimiters.
pub fn stream(&self) -> TokenStream {
self.tts.clone().into()
}
}

@eddyb
Copy link
Member

eddyb commented Sep 24, 2018

How does DelimSpan compare to what {open,close}_tt compute?
Also, {open,close}_token could be methods on DelimToken itself.
So overall, losing Delimited doesn't feel too bad.

@dtolnay
Copy link
Member Author

dtolnay commented Sep 24, 2018

The {open,close}_tt methods are passed in a Span. DelimSpan is where the span passed in comes from.

@dtolnay dtolnay added the I-compiletime Issue: Problems and improvements with respect to compile times. label Oct 1, 2018
@whitfin
Copy link
Contributor

whitfin commented May 7, 2019

@dtolnay I was going to take a look at this but it seems it might have already been done (and so this can be closed). The current TokenTree::Delimited looks like this:

pub enum TokenTree {
    Token(Span, token::Token),
    Delimited(DelimSpan, DelimToken, TokenStream),
}

@dtolnay
Copy link
Member Author

dtolnay commented May 7, 2019

Thanks, looks like this was fixed in #56369.

@dtolnay dtolnay closed this as completed May 7, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
I-compiletime Issue: Problems and improvements with respect to compile times.
Projects
None yet
Development

No branches or pull requests

3 participants