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

Dependencies cleanup #128

Merged
merged 10 commits into from
Oct 3, 2019
Merged

Dependencies cleanup #128

merged 10 commits into from
Oct 3, 2019

Conversation

lqd
Copy link
Member

@lqd lqd commented Sep 19, 2019

This PR cleans up the dependencies, to reduce compile times on a clean build:

While CLI arguments handling becomes less automated and slightly more verbose, this all reduces compile times greatly: a cargo build --release clean build (on a beefy machine) goes from 52s to 17s (as the number of dependencies goes from 60 to 12).

The dev-dependencies haven't changed much, maybe 30 crates less than before and 20s faster, but I'll see what I can do with these later.

maybe we should remove this one
@lqd
Copy link
Member Author

lqd commented Sep 21, 2019

And just because I wanted to play with cargo's new -Z timings, here's how the build timings look for this PR (on an old laptop and in debug mode, so the durations are different from the PR description — still a 3x difference):

This makes me want to check whether we require the default-features of petgraph which also themselves require dependencies (even though, we could easily remove it completely if we wanted to: we already did graph reduction in a way similar to what is needed by the --dump-liveness-graph feature, in the CFG compression PR).

@amandasystems
Copy link
Contributor

This makes me want to check whether we require the default-features of petgraph which also themselves require dependencies (even though, we could easily remove it completely if we wanted to: we already did graph reduction in a way similar to what is needed by the --dump-liveness-graph feature, in the CFG compression PR).

I think it makes total sense to have a centralised graph-compression implementation, because as you point out it is definitely the same logic. I also think it makes sense to drop --dump-liveness-graph, as it was something I used when working with the liveness implementation, and I suspect it's done now.

The way I dreamed this up is that the compression algorithm would just be a BFS that takes a function acting as a "are these two equal" predicate for each prospective pair of nodes to merge. That way the logic of determining the equality classes of nodes can be abstracted out and re-used whenever we need a graph compression over anything.

@nikomatsakis nikomatsakis merged commit c78359a into rust-lang:master Oct 3, 2019
@lqd lqd deleted the into_the_deps branch October 3, 2019 12:39
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