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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Deps: prefer sub-crates of futures #935

Merged
merged 6 commits into from Aug 16, 2022
Merged

Deps: prefer sub-crates of futures #935

merged 6 commits into from Aug 16, 2022

Conversation

BastiDood
Copy link
Contributor

@BastiDood BastiDood commented Aug 14, 2022

This PR attempts to shorten compilation times by preferring dependencies on the sub-crates of futures rather than the entire futures crate itself. The changes I've made practically slim down the default features (of futures) to just two crates: futures-util and futures-channel.

To make CI pass, I also needed to fix some Clippy warnings. Most of them were mechanical changes.

With that said, I don't expect any breaking changes. At the very least, there should be a slight improvement in compilation times. Thanks! 馃帀

@BastiDood BastiDood changed the title Deps: prefer sub crates of futures Deps: prefer sub-crates of futures Aug 14, 2022
Comment on lines +76 to +78
type ConfigCallback =
dyn Fn(&mut ConnectConfiguration, &str) -> Result<(), ErrorStack> + Sync + Send;

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Clippy complains if we don't use a type alias here. To be fair, it is a pretty lengthy type declaration. 馃槄

@sfackler
Copy link
Owner

How much does this improve build times?

@BastiDood
Copy link
Contributor Author

System Information

The following measurements have been tested on a laptop with the following specs:

Rust Version OS CPU RAM
1.63.0 (Stable) Windows 10 21H2 (19044.1889) Intel Core i7-10750H @ 2.60GHz (12 CPUs) 8192 MB

I then ran the following commands and recorded the (cold) compilation times reported by Cargo:

cargo clean                # clean up old artifacts
cargo build -p postgres    # only compile `postgres` on debug

cargo clean                # clean up old artifacts
cargo build -p postgres -r # only compile `postgres` on release

Native Windows

On native Windows, the build times improved ever so slightly on debug builds. For the release builds, however, I must say that the improvements may still be within the realm of noise.

However, there is an improvement in the number of crates needed to compile. Before the PR, a Windows build required 105 crates. Now, it only requires 103 crates. This is likely due to the absence of the futures-executor crate in the production dependencies.

Version Debug Release
Before PR 27.38s 34.71s
After PR 25.06s 34.58s

Linux

I also measured on an Ubuntu 22.04.1 build (via Windows Subsystem for Linux). There is also a slight improvement on debug builds. Release builds remained more or less the same.

Regarding the number of crates compiled, the Linux side had two fewer crates to compile (from 101 to 99). This difference is consistent with native Windows. Again, this is likely due to the absence of the futures-executor crate.

Version Debug Release
Before PR 17.20s 23.52s
After PR 16.46s 23.29s

Conclusion

There is a slight improvement in cold debug builds. In both Windows and Linux (via WSL), we reduced the number of compiled crates by two. For projects with larger dependency trees, removing futures-executor from our direct dependencies will allow Cargo to compile postgres earlier in the topological sort鈥攑erhaps even in parallel with futures-executor!

@sfackler sfackler merged commit 8f955eb into sfackler:master Aug 16, 2022
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.

None yet

2 participants