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

Use attohttpc instead of reqwest for fetching #733

Merged
merged 1 commit into from
May 15, 2020

Conversation

charlespierce
Copy link
Contributor

@charlespierce charlespierce commented May 6, 2020

Info

  • As originally found by @joshuarli in Investigate build bloat #536, we don't need all the async features of reqwest in Volta, since all of our HTTP access is synchronous.
  • Unfortunately, while reqwest provides a synchronous API, it still uses the async engine and task executor (tokio) under the hood.
  • Compiling that bloats our build times and executable size, by bringing in an entire async executor when we don't actually use it.
  • attohttpc is a purely-synchronous HTTP Request client with a very similar API to reqwest.

Changes

  • Removed reqwest and added attohttpc to all of the appropriate crates.
  • Upgraded hyperx to 1.0.0, to match the transitive dependency on http
  • Updated the code to fit attohttpcs slightly different API

Notes

  • This results in a ~25% reduction in the executable size for volta, and a ~30% reduction in the executable size for volta-shim.
  • Anecdotally, this reduced the time for a clean cargo build --release by ~ 22% (from 6m12s down to 4m50s).
  • For performance, again anecdotally, I see no noticeable difference for fetching a version of Node, though I do get a statistically significant speedup on the hot path (likely a result of the smaller binary).

@joshuarli
Copy link
Contributor

Oh neat, I didn't know at all about hyperx back when I made a shitty attempt at porting old header stuff to new header stuff to upgrade reqwest. Seems like it made things a lot easier.

@charlespierce
Copy link
Contributor Author

Yeah, definitely, the hyperx stuff made it a lot easier, not having to re-implement the header parsing. I didn’t know about the crate either until a day or two ago 😄

@charlespierce charlespierce marked this pull request as ready for review May 6, 2020 17:36
@charlespierce charlespierce changed the title [Experimental] Use attohttpc instead of reqwest for fetching Use attohttpc instead of reqwest for fetching May 11, 2020
Copy link
Contributor

@chriskrycho chriskrycho left a comment

Choose a reason for hiding this comment

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

⚡ ship it! Nice clean refactor, and cutting down compile time and build size both? ⚡ ❗

@charlespierce charlespierce merged commit 8e5a722 into volta-cli:master May 15, 2020
@charlespierce charlespierce deleted the use_attohttpc branch May 15, 2020 17:15
@gregjopa gregjopa mentioned this pull request Sep 4, 2020
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