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 #162

Closed
wants to merge 2 commits into from
Closed

Conversation

rylev
Copy link

@rylev rylev commented Jul 6, 2020

This addresses #160 along with dtolnay/proc-macro2#239.

The bench mark runs more than twice as fast, and my own internal benchmarking shows improvements, but there are limitations to potential performance gains discussed in the proc-macro2 pull request.

@jplatte
Copy link

jplatte commented Aug 17, 2021

I'd be very interested in trying this out via [patch] but unfortunately sth. in my dependency tree has a minimum version requirement for quote of 1.0.8. Can you rebase this (and dtolnay/proc-macro2#239)? Might also increase the likelyhood of having it merged. (though general lack of maintainer attention is clearly the larger problem)

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.

(Rebased: 0c1f124 + dtolnay/proc-macro2@9a035f2.)

According to the quote! benchmark in https://github.com/dtolnay/quote/tree/8f0d3c19eb1caba32965b551e99c00b68568cec5/benches, it looks like this pair of changes ends up being 65% slower in the proc macro case than current master. I don't think this is going to be the right tradeoff for a crate that is predominantly used in proc macros.

Thanks anyway for putting this together! I still consider it very likely possible to do string buffering in a way that makes quote faster in both macro and non-macro context, because calls over the compiler's proc macro bridge are moderately expensive and can be reduced by batching in a string buffer that's parsed all in one call over the bridge.

I'll close the PR to preserve this version of things but am open to another PR that does not regress the proc macro use case.

@dtolnay dtolnay closed this Dec 26, 2021
@rylev rylev deleted the stringify 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.

3 participants