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

String-buffer backed TokenStream #239

Closed
wants to merge 8 commits into from
Closed

Conversation

rylev
Copy link

@rylev rylev commented Jul 3, 2020

This is the first steps in addressing dtolnay/quote#160

This changes TokenStreams to keep no spanned tokens as plain strings and only convert them to imp::TokenStream's when necessary.

This doesn't make a difference in the proc_macro benchmarks as that is still using older APIs that go through the proc_macro bridge, but it cuts quote's benchmark time in half. This makes my own crate's (winrt) benchmark ~35% faster, though this is not as fast as the squote experiment which did not support spanned tokens at all. The reason for this is because delimited tokens must be balanced and so we have to treat groups differently even if they only contain non-spanned inner tokens. This prevents us from simply treating groups like strings and folding them into string backed token streams.

@rylev
Copy link
Author

rylev commented Aug 19, 2020

@dtolnay would you be able to take a look at this?

Copy link
Owner

@dtolnay dtolnay left a comment

Choose a reason for hiding this comment

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

Closing along with the quote PR since this seems to make the proc macro use case 65% slower, which is not the right tradeoff for a crate that is predominantly used in proc macros — see dtolnay/quote#162 (review).

Thanks anyway!

@dtolnay dtolnay closed this Dec 26, 2021
@rylev rylev deleted the string-backed branch January 3, 2022 12:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants