Skip to content

/cargo

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

Speed up noop builds by 10x #1668

Merged
merged 9 commits into from Jun 3, 2015

Conversation

Reviewers
No reviews
Assignees

@huonw huonw

Labels
None yet
Projects
None yet
Milestone
No milestone
5 participants
@alexcrichton @rust-highfive @huonw @bluss @bors
@alexcrichton
Copy link
Member

alexcrichton commented May 31, 2015

Playing around with Servo recently I realized that a "noop build" (e.g. cargo build had no work to do) took about 5s to complete on my machine! I wasn't super happy with this performance, so I investigated the performance and applied a number of optimizations as part of this PR:

  • Primarily, algorithmic changes were made to not traverse the dependency graph too often. There were previously two code paths where each node in the graph would traverse its entire subgraph, causing excessively high runtimes.
  • Many minor changes were made to reduce allocations and in general cache results between invocations.
  • The Rust version in use was updated to pick up the recent change to look at DT_DIR for DirEntry::file_type

Overall this PR improves a noop build of Servo from 5s to 0.5s on my machine.

@rust-highfive rust-highfive assigned huonw May 31, 2015

@rust-highfive

This comment has been minimized.

Sign in to view
Copy link

rust-highfive commented May 31, 2015

r? @huonw

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

This comment has been minimized.

Sign in to view
Copy link
Member

huonw commented Jun 1, 2015

Travis failed in a way that looks non-spurious.
@alexcrichton

This comment has been minimized.

Sign in to view
Copy link
Member Author

alexcrichton commented Jun 1, 2015

Ah that's actually just a flaky test on OSX, I've been meaning to track that down for some time now...

@alexcrichton alexcrichton force-pushed the alexcrichton:perf branch from 6bb9d6a to 0a23659 Jun 1, 2015

@bluss

This comment has been minimized.

Sign in to view
Copy link
Contributor

bluss commented Jun 2, 2015

This is great ⭐️
@huonw

This comment has been minimized.

Sign in to view
Copy link
Member

huonw commented Jun 3, 2015

@bors r+
@bors

This comment has been minimized.

Sign in to view
Copy link
Contributor

bors commented Jun 3, 2015

📌 Commit 0a23659 has been approved by huonw
@bors

This comment has been minimized.

Sign in to view
Copy link
Contributor

bors commented Jun 3, 2015

⌛️ Testing commit 0a23659 with merge bfb9469...

bors added a commit that referenced this pull request Jun 3, 2015

@bors
Auto merge of #1668 - alexcrichton:perf, r=huonw 
Playing around with Servo recently I realized that a "noop build" (e.g. `cargo build` had no work to do) took about 5s to complete on my machine! I wasn't super happy with this performance, so I investigated the performance and applied a number of optimizations as part of this PR:

* Primarily, algorithmic changes were made to not traverse the dependency graph too often. There were previously two code paths where each node in the graph would traverse its entire subgraph, causing excessively high runtimes.
* Many minor changes were made to reduce allocations and in general cache results between invocations.
* The Rust version in use was updated to pick up the recent change to look at `DT_DIR` for `DirEntry::file_type`

Overall this PR improves a noop build of Servo from 5s to 0.5s on my machine.
bfb9469
@bors

This comment has been minimized.

Sign in to view
Copy link
Contributor

bors commented Jun 3, 2015

💔 Test failed - cargo-mac-32

alexcrichton added some commits May 29, 2015

@alexcrichton
Implement caching among fingerprint resolutions 
This commit adds support for caching the resolved value of a fingerprint among
calls to `resolve`. Due to the recursive nature of a fingerprint, it means that
`resolve` is currently executed a very large number of times for packages which
at transitively dependend up on many times. By caching the returned value of a
fingerprint we're able to prevent extraneous calls into the filesystem or
extraneous hashing.

This also adds an `Arc` to share a `Fingerprint` among all its dependencies as
each fingerprint stores a list of fingerprints that it depends on, and if
they're not shared then the cache isn't exactly the most helpful!

For a noop `cargo build` on Servo (e.g. everything was already built), this
decreased Cargo's runtime from 5s to ~4.5s
7762289
@alexcrichton
Don't recurse so often in the dependency graph 
Prior to this commit the entire dependency tree would be traversed many, many
times as the short circuits weren't applying fast enough. This commit modifies
the recursion to only happen if the target in question was actually compiled.

This commit saw a noop build time for Servo drop from 4.5s to 2.5s.
450e81d
@alexcrichton
Switch from a custom MTime to the filetime crate 
The crate provides the same functionality necessary for Cargo, but it's
maintained elsewhere now.
7d49029
@alexcrichton
Update the Rust snapshot used 
At least one important I/O related perf fix has landed on nightly (reading
`DT_DIR` for `DirEntry::file_type`) and there are a few other assorted bug fixes
which are being picked up.
81ecf8b
@alexcrichton
Don't allocate in PackageId::hash 
This hash function is called an enormous number of times in `cargo_rustc`, so
optimizing it is fairly important, and it's also quite trivial to not allocate a
string at all (`semver::Version::hash` already does everything we need).
499f485
@alexcrichton
Optimize crawl_build_deps to not re-traverse deps 
This function previously, for each dependency, traversed the entire dependency
graph. This ended up taking quite a bit of time as a huge amount of hashing was
being done (each of which is somewhat nontrivial). This commit alters the logic
to instead precompute a table of all the native dependencies that each crate
will need, traversing the dependency graph only once instead of repeatedly.

This commit takes a noop build of Servo from 2.1s to 0.4s
5a953eb
@alexcrichton
Optimize SourceId::cmp a bit
6d7ce09
@alexcrichton
Cache the canonical URL for a source 
This avoids reallocating and recalculating it on each call to
SourceIdInner::{eq, hash}, which are called quite often in the backend.
f916188

@alexcrichton alexcrichton force-pushed the alexcrichton:perf branch from 0a23659 to b7e2a58 Jun 3, 2015

@alexcrichton

This comment has been minimized.

Sign in to view
Copy link
Member Author

alexcrichton commented Jun 3, 2015

@bors: r=huonw b7e2a58
@bors

This comment has been minimized.

Sign in to view
Copy link
Contributor

bors commented Jun 3, 2015

⌛️ Testing commit b7e2a58 with merge 1c26608...

bors added a commit that referenced this pull request Jun 3, 2015

@bors
Auto merge of #1668 - alexcrichton:perf, r=huonw 
Playing around with Servo recently I realized that a "noop build" (e.g. `cargo build` had no work to do) took about 5s to complete on my machine! I wasn't super happy with this performance, so I investigated the performance and applied a number of optimizations as part of this PR:

* Primarily, algorithmic changes were made to not traverse the dependency graph too often. There were previously two code paths where each node in the graph would traverse its entire subgraph, causing excessively high runtimes.
* Many minor changes were made to reduce allocations and in general cache results between invocations.
* The Rust version in use was updated to pick up the recent change to look at `DT_DIR` for `DirEntry::file_type`

Overall this PR improves a noop build of Servo from 5s to 0.5s on my machine.
1c26608
@bors

This comment has been minimized.

Sign in to view
Copy link
Contributor

bors commented Jun 3, 2015

💔 Test failed - cargo-mac-32
@alexcrichton
Update rust-url to improve hash/cmp performance 
Both no longer require allocations!
Loading status checks…
e5a870b

@alexcrichton alexcrichton force-pushed the alexcrichton:perf branch from b7e2a58 to e5a870b Jun 3, 2015

@alexcrichton

This comment has been minimized.

Sign in to view
Copy link
Member Author

alexcrichton commented Jun 3, 2015

@bors: r+ e5a870b
@bors

This comment has been minimized.

Sign in to view
Copy link
Contributor

bors commented Jun 3, 2015

⌛️ Testing commit e5a870b with merge 69c2af2...

bors added a commit that referenced this pull request Jun 3, 2015

@bors
Auto merge of #1668 - alexcrichton:perf, r=alexcrichton 
Playing around with Servo recently I realized that a "noop build" (e.g. `cargo build` had no work to do) took about 5s to complete on my machine! I wasn't super happy with this performance, so I investigated the performance and applied a number of optimizations as part of this PR:

* Primarily, algorithmic changes were made to not traverse the dependency graph too often. There were previously two code paths where each node in the graph would traverse its entire subgraph, causing excessively high runtimes.
* Many minor changes were made to reduce allocations and in general cache results between invocations.
* The Rust version in use was updated to pick up the recent change to look at `DT_DIR` for `DirEntry::file_type`

Overall this PR improves a noop build of Servo from 5s to 0.5s on my machine.
Loading status checks…
69c2af2
@bors

This comment has been minimized.

Sign in to view
Copy link
Contributor

bors commented Jun 3, 2015

☀️ Test successful - cargo-linux-32, cargo-linux-64, cargo-mac-32, cargo-mac-64, cargo-win-gnu-32, cargo-win-gnu-64

@bors bors merged commit e5a870b into rust-lang:master Jun 3, 2015
2 checks passed

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
homu Test successful
Details

@alexcrichton alexcrichton deleted the alexcrichton:perf branch Jun 15, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.