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

Deps: significantly improve compilation times by removing the futures crate #11

Merged
merged 8 commits into from
May 18, 2022
Merged

Deps: significantly improve compilation times by removing the futures crate #11

merged 8 commits into from
May 18, 2022

Conversation

BastiDood
Copy link
Contributor

@BastiDood BastiDood commented May 18, 2022

This PR attempts to improve compilation times by minimizing our dependency on the heavy futures crate, which compiles the futures-executor crate by default. In my fork, the same GitHub Actions workflows dropped by ~20 seconds. This is great!

I did face some really strange issues that required temporary workarounds with the devDependencies. But as far as the user is concerned, they should not be affected by these changes. More on that in a separate review comment. Update: see #11 (comment).

Besides that, everything should be alright! 👍

BastiDood added 8 commits May 18, 2022 18:09
For some reason, using the `futures-channel` crate over the `futures`
crate causes compilation errors. This is very strange because the
`futures::channel` module from the `futures` crate is literally just
re-exported from the `futures-channel` crate!

Despite this, the code fails to compile whenever I use `futures-channel`
instead. As a temporary workaround, we use the full `futures` crate in
the tests. However, when the user downloads the crate, they will still
get the minimal sub-crates (not the full `futures` crate).
@BastiDood
Copy link
Contributor Author

Strange that the GitHub Actions for this PR ran longer than usual. But I can assure you that there have been significant improvements when I ran the workflows in my fork. In particular, the cargo check step has been shortened to a runtime of 1:07. The overall CI time became 1:37.

@MattiasBuelens
Copy link
Owner

Thank you, this look great! 😄 I didn't know futures-util existed as a separate crate, but I'm happy it does and it can reduce compilation times. 🙂

I'll give this a proper review later today.

@MattiasBuelens MattiasBuelens merged commit b33fcbc into MattiasBuelens:master May 18, 2022
@BastiDood BastiDood deleted the deps/remove-futures-crate branch May 18, 2022 17:29
@MattiasBuelens MattiasBuelens added this to the v0.2.3 milestone May 18, 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.

2 participants