Skip to content

Conversation

@illicitonion
Copy link
Owner

For large (thousands of variants) enums, this drops the time to generate
from minutes to seconds.

I don't love this, but it's a significant speedup, and the code we're
generating is pretty simple...

I'll file a bug against proc-quote about the perf.

Fixes #13 /cc @anlumo

@danielhenrymantilla
Copy link
Collaborator

Oh, that's both interesting and disappointing 😅
Since the performance gain is huge, we'll have to settle for this version. I am still curious though: since you have already done some benchmarks, could you also benchmark using ::quote::quote! rather than ::proc_quote::quote! ? (c.f. #15)

@illicitonion
Copy link
Owner Author

I checked - quote took 88 seconds, compared to proc-quote's 94. I'm going to merge #14 (and switch to regular quote), then rebase and merge this on top, so that it will be easy for us to switch back if the perf issues get resolved :)

For large (thousands of variants) enums, this drops the time to generate
from minutes to seconds.

I don't love this, but it's a significant speedup, and the code we're
generating is pretty simple...

I'll file a bug against proc-quote about the perf.
@dtolnay
Copy link

dtolnay commented Oct 4, 2019

The slowness is a bug in the compiler's libproc_macro: rust-lang/rust#65080.

I opened dtolnay/proc-macro2#199 with a workaround that drops the cargo expand time of https://github.com/illicitonion/num_enum_regression_13 (using quote) from 118 seconds to 2.0 seconds on my machine.

@danielhenrymantilla
Copy link
Collaborator

danielhenrymantilla commented Oct 4, 2019

Wow, thanks for stepping in, @dtolnay. The change you have suggested to proc-macro2 is great, it should be faster to get that going than the original fix on proc-macro.

So the best course of action for this PR is to wait for a quote release (or git rev) to depend on this patched version of proc-macro2, right?

@dtolnay
Copy link

dtolnay commented Oct 4, 2019

I published the workaround in proc-macro2 so the performance should already be fixed once you run cargo update.

@illicitonion
Copy link
Owner Author

Closing - fix is pushed to transitive dep :)

@illicitonion illicitonion deleted the speed-from-strings branch March 17, 2020 09:47
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.

Performance Issue on Large enums

4 participants