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

Unnaturally slow expansion on generated source #43573

Closed
alexcrichton opened this issue Jul 31, 2017 · 12 comments · Fixed by #43584
Closed

Unnaturally slow expansion on generated source #43573

alexcrichton opened this issue Jul 31, 2017 · 12 comments · Fixed by #43584
Labels
I-compiletime Issue: Problems and improvements with respect to compile times. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@alexcrichton
Copy link
Member

In investigating #43572 I was generating some source code, and using the shell script listed in #43572 we can see:

$ rustc +nightly -V
rustc 1.21.0-nightly (aac223f4f 2017-07-30)
$ sh foo.sh
$ rustc +nightly lots-of-unused.rs -Z time-passes 2>&1 | grep 'expansion'
time: 1.567; rss: 110MB expansion

That's a lot of time to expand a crate that doesn't have any macros in it!

@alexcrichton alexcrichton added I-compiletime Issue: Problems and improvements with respect to compile times. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jul 31, 2017
@alexcrichton
Copy link
Member Author

cc @jseyfried, you may be interested in this!

@alexcrichton
Copy link
Member Author

A profile of this looks like almost all the time is spent in rustc::hir::map::definitions::Definitions::create_def_with_parent

In which case, cc @michaelwoerister!

@alexcrichton
Copy link
Member Author

Apparently most time is spent computing these hashes

@alexcrichton
Copy link
Member Author

called ~38k times

@jseyfried
Copy link
Contributor

Since we need to build the module graph and resolve imports during expansion for macros 2.0, we must construct node ids and def ids throughout expansion (i.e. interleaved arbitrarily many times until we hit fix-point).

Ideally we'd keep running totals of the time spent assigning node ids, assigning def ids, etc. for more accurate profiling information within the expansion step.

@alexcrichton
Copy link
Member Author

@jseyfried in theory though isn't the fixed point for this crate the crate itself? (e.g. there's no macros here). Or are the def ids persisted for the rest of compilation, so "blaming" expansion isn't quite correct here?

@jseyfried
Copy link
Contributor

jseyfried commented Aug 1, 2017

@alexcrichton Yeah, the def ids are persisted for the rest of compilation. We could move the "first round" of def id collection (in which we collect the raw, unexpanded source) outside of expansion, but that would introduce more code complexity and wouldn't count the time we spend collecting def ids for AST that comes from macro expansions.

@alexcrichton
Copy link
Member Author

Ah ok, nah it seems fine! I think there's a number of cases where -Z time-passes is attributing time in the wrong place nowadays...

Still it seems odd that 30-50k invocations of this function would take so long to execute?

@michaelwoerister
Copy link
Member

@jseyfried Do I understand this correctly that all DefIds generated are actually used, right? There are no "temporary" DefIds?

Still it seems odd that 30-50k invocations of this function would take so long to execute?

Each of these invocations creates a Blake2 hasher. I'd be interested how the performance looks with other hashing algorithms. I'd like to switch to something faster anyway: #41215

@arielb1
Copy link
Contributor

arielb1 commented Aug 1, 2017

@alexcrichton

Are you sure this isn't the other quadratic case fixed in #43584?

@alexcrichton
Copy link
Member Author

It may very well be! Feel free to close this w/ that PR landing, I'll verify to be sure but I'd imagine you're correct in that it's fixed by #43584

@alexcrichton
Copy link
Member Author

er, didn't mean to close just yet

bors added a commit that referenced this issue Aug 2, 2017
Fix quadratic performance with lots of use statements

This fixes 2 problems that caused quadratic performance when lots of use-statements were present. After this patch, performance is linear (and very fast) even with 1M uses.

Fixes #43572.
Fixes #43573.

r? @eddyb
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. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants