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

Fix exponentiality in depend_on_deps_of_deps. #7062

Merged
merged 2 commits into from
Jun 25, 2019
Merged

Conversation

goffrie
Copy link
Contributor

@goffrie goffrie commented Jun 22, 2019

With CARGO_BUILD_PIPELINING=true, cargo was spending a long time (15 seconds) before starting any compilation. That is caused by a naive graph traversal in depend_on_deps_of_deps. Instead, let's make sure not to keep traversing the same deps. With this patch, things are fast again.

@rust-highfive
Copy link

r? @ehuss

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jun 22, 2019
@Eh2406
Copy link
Contributor

Eh2406 commented Jun 23, 2019

Drive by comment, should this have a test?

@alexcrichton
Copy link
Member

Thanks for this! I think that TODO is actually quite old and perhaps not ready to be fully closed out, could that be backed out from this change? Otherwise this looks reasonable to me.

If we can have a test that'd be great, but I'd assume that this would require a pretty large crate graph and/or this would very quickly come up when pipelining was turned on so I don't think that a test is critical.

@goffrie
Copy link
Contributor Author

goffrie commented Jun 24, 2019

I have undone the other change, and I added a test targeting the bad behaviour. I confirmed that without my patch, the test takes a very long time (at least 60 seconds, but it should internally traverse something like 2^30 deps), but now finishes very quickly.

@alexcrichton
Copy link
Member

@bors: r+

Thanks!

@bors
Copy link
Contributor

bors commented Jun 25, 2019

📌 Commit 15e0802 has been approved by alexcrichton

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 25, 2019
@bors
Copy link
Contributor

bors commented Jun 25, 2019

⌛ Testing commit 15e0802 with merge 83d086d...

bors added a commit that referenced this pull request Jun 25, 2019
Fix exponentiality in depend_on_deps_of_deps.

With `CARGO_BUILD_PIPELINING=true`, cargo was spending a long time (15 seconds) before starting any compilation. That is caused by a naive graph traversal in `depend_on_deps_of_deps`. Instead, let's make sure not to keep traversing the same deps. With this patch, things are fast again.
@bors
Copy link
Contributor

bors commented Jun 25, 2019

☀️ Test successful - checks-travis, status-appveyor
Approved by: alexcrichton
Pushing 83d086d to master...

@bors bors merged commit 15e0802 into rust-lang:master Jun 25, 2019
bors added a commit to rust-lang/rust that referenced this pull request Jul 10, 2019
Update cargo

Update cargo

12 commits in 4c1fa54d10f58d69ac9ff55be68e1b1c25ecb816..677a180f4c8ca1dcef594f68dd0e63e4f08621f5
2019-06-24 11:24:18 +0000 to 2019-07-08 13:43:02 +0000
- Update dependencies (rust-lang/cargo#7106)
- Fix overwriting .d file for binary with dSYM on apple targets. (rust-lang/cargo#7057)
- Update changelog. (rust-lang/cargo#7102)
- Revert "Fix 'getting started' internal links" (rust-lang/cargo#7099)
- Bump to 0.39.0 (rust-lang/cargo#7100)
- Update dependency.rs (rust-lang/cargo#7098)
- Typo (rust-lang/cargo#7097)
- Fix 'getting started' internal links (rust-lang/cargo#7093)
- Fix misleading comment in testsuite (rust-lang/cargo#7090)
- improve uncommitted changes cargo-package message (rust-lang/cargo#7083)
- Clean environment when git-fetch-with-cli is used. (rust-lang/cargo#7082)
- Fix exponentiality in depend_on_deps_of_deps. (rust-lang/cargo#7062)
@ehuss ehuss added this to the 1.38.0 milestone Feb 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants