-
Notifications
You must be signed in to change notification settings - Fork 905
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
Stream unpacking of source distribution downloads #1157
Conversation
8709d74
to
5768543
Compare
5768543
to
79d643d
Compare
@@ -19,6 +19,9 @@ license = "MIT OR Apache-2.0" | |||
[workspace.dependencies] | |||
anstream = { version = "0.6.5" } | |||
anyhow = { version = "1.0.79" } | |||
async-compression = { version = "0.4.6" } | |||
async-std = {version = "1.6.0" } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm a bit worried about the async-std dep, it's abandoned and could clash with tokio(?)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is your source for async-std
being abandoned?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can migrate to https://crates.io/crates/tokio-tar which is a fork for Tokio. Or I can look into forking async-tar
to land this PR: dignifiedquire/async-tar#41.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
## Summary `tokio_tar` is a fork of `async_tar` that uses Tokio instead of `async-std`. Using it removes a significant dependency from our tree. (There is an open PR (dignifiedquire/async-tar#41) in `async-tar` to add Tokio support, but it's over a year old.) See: #1157 (comment).
This PR migrates our source distribution downloads to unzip as we stream, similar to our approach for wheels.
In my testing, this showed a consistent speedup (e.g., 6% here for a few representative source distributions):