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

refactor(lib): Switch from pin-project to pin-project-lite (again) #2566

Merged
merged 1 commit into from Jun 4, 2021

Conversation

jplatte
Copy link
Contributor

@jplatte jplatte commented Jun 2, 2021

Second try, turned out to require a bunch more changes in src/client/conn.rs and src/server/conn.rs due to the ProtoClient and ProtoServer enums which previously had cfg-conditional variants (which aren't supported by pin-project-lite). Now the variants that previously were disabled by cfg(not(feature = "http1")) and cfg(not(feature = "http2")) are instead made to be uninhabited.

The code would be a decent amount simpler if the non-exhaustive variants could be omitted in matches (rust-lang/rust#51085).

@jplatte jplatte changed the title refactor(lib): Switch from pin-project to pin-project-lite again refactor(lib): Switch from pin-project to pin-project-lite (again) Jun 2, 2021
Copy link
Member

@seanmonstar seanmonstar left a comment

Choose a reason for hiding this comment

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

Thanks so much, excellent work!

src/client/conn.rs Outdated Show resolved Hide resolved
src/server/conn.rs Outdated Show resolved Hide resolved
Copy link
Member

@seanmonstar seanmonstar left a comment

Choose a reason for hiding this comment

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

<3

@seanmonstar
Copy link
Member

Note the practical affects of this PR:

  • Dependency count with --features full dropped from 65 to 55.
  • Time to compile after a clean dropped from 48s to 35s (on a pretty underpowered VM).

🎉

@seanmonstar seanmonstar merged commit 6a6a240 into hyperium:master Jun 4, 2021
@jplatte jplatte deleted the pin-project-lite branch June 4, 2021 21:57
@jplatte
Copy link
Contributor Author

jplatte commented Jun 4, 2021

I'm actually a bit surprised about the dependency diff. pin-project should "only" pull in pin-project-internal > syn > quote > proc-macro2 > unicode-xid AFAICT.

@seanmonstar
Copy link
Member

Hm, I think you're right. I just based that number off making a new crate, adding hyper as a dependency (to not get any dev-dependencies), and then recording how it says Building [ ] 1/55 and then Building [ ] 1/65. Probably 1 of those is the empty new lib, but otherwise I'm not sure...

@seanmonstar
Copy link
Member

Ah, paying more attention, it's because the build.rs step of a dependency is counted in there too. And maybe another step related to proc macro libs?

@jplatte
Copy link
Contributor Author

jplatte commented Jun 6, 2021

Will there be a patch release with this soon?

@seanmonstar
Copy link
Member

Yep, just released v0.14.9.

BenxiangGe pushed a commit to BenxiangGe/hyper that referenced this pull request Jul 26, 2021
…2566)

Note the practical affects of this change:

- Dependency count with --features full dropped from 65 to 55.
- Time to compile after a clean dropped from 48s to 35s (on a pretty underpowered VM).

Closes hyperium#2388
edmorley added a commit to edmorley/bollard that referenced this pull request Feb 28, 2022
This is another small step towards reducing the size of bollard's
dependency tree :-)

`tokio` and `hyper` have both switched from `pin-project` crate
to the lighter-weight `pin-project-lite`:
tokio-rs/tokio#1778
hyperium/hyper#2566

This does the same for bollard. For the differences between the
two crates, see:
https://docs.rs/pin-project-lite/0.2.8/pin_project_lite/#pin-project-vs-pin-project-lite

Note: The full advantage of this won't be seen until a new
`hyperlocal` release exists that contains:
softprops/hyperlocal#54

...and bollard updates to that release, so that `pin-project` can be
fully dropped from the dependency tree.
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