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.

Sign up for GitHub

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

Jump to bottom

Dependency segmentation #54

Merged
merged 4 commits into from
Jan 10, 2022
Merged

Dependency segmentation #54

merged 4 commits into from
Jan 10, 2022

Conversation

onalante-msft
Copy link
Contributor

This PR is technically comprised of three changes. Since the changes are small individually, they are bundled together here. I can split the changes into separate PRs if that is preferred.

The main motivation of this PR is to segment hyper's server and client features into hyperlocal's server and client features.
This helps reduce compile times for dependents only needing one of the features. To further reduce compile times, this PR also exchanges pin-project for pin-project-lite and removes the futures-util dependency: pin-project-lite is used over pin-project in tokio, and Rust std::future is now standard enough to accomplish what is included from futures-util.

Some basic timings (cargo clean before each run):

command base change
cargo build --no-default-features --features=client 18.04s 15.58s
cargo build --no-default-features --features=server 17.84s 15.32s
cargo build --all-features 18.88s 16.19s

@softprops softprops merged commit 7a2cbb8 into softprops:main Jan 10, 2022
@softprops
Copy link
Owner

Excellent and thank you!

@onalante-msft
Copy link
Contributor Author

onalante-msft commented Jan 10, 2022 •
edited
Loading

Sorry, I did not push a formatting commit. I will open another PR to fix the formatting errors ASAP. It will also include some documentation updates and clippy checks.

@onalante-msft onalante-msft mentioned this pull request Jan 10, 2022
Merged
edmorley added a commit to edmorley/hyperlocal that referenced this pull request Feb 26, 2022
@edmorley
Document that the client/server features are not opt-in
9548b0f 
Adds documentation for the switch to the `client` and `server` features now
being opt-in, rather than enabled by default (see softprops#54).

I experimented with also enabling the nightly-only `doc_cfg` / `doc_auto_cfg`
features (see rust-lang/rust/issues/43781, which can be used on `docs.rs` since
it builds docs with nightly), however:
- the features don't currently work well with the re-exports in the crate root,
  so require every struct/trait to be manually annotated.
- the rendering isn't great - the crate landing page wrapping ended up broken

So for now it seems best to not use that.

I did adjust the `docs.rs` config in `Cargo.toml` to ensure docs are actually
generated for all features, since previously the docs were almost empty.

I also fixed a few typos / markdown syntax errors.
edmorley added a commit to edmorley/hyperlocal that referenced this pull request Feb 26, 2022
@edmorley
Document that the client/server features are now opt-in
afeaf2e 
Adds documentation for the switch to the `client` and `server` features now
being opt-in, rather than enabled by default (see softprops#54).

I experimented with also enabling the nightly-only `doc_cfg` / `doc_auto_cfg`
features (see rust-lang/rust/issues/43781, which can be used on `docs.rs` since
it builds docs with nightly), however:
- the features don't currently work well with the re-exports in the crate root,
  so require every struct/trait to be manually annotated.
- the rendering isn't great - the crate landing page wrapping ended up broken

So for now it seems best to not use that.

I did adjust the `docs.rs` config in `Cargo.toml` to ensure docs are actually
generated for all features, since previously the docs were almost empty.

I also fixed a few typos / markdown syntax errors.
@edmorley edmorley mentioned this pull request Feb 26, 2022
Closed
edmorley added a commit to edmorley/bollard that referenced this pull request Feb 28, 2022
@edmorley
Switch from pin-project to pin-project-lite
088e582 
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.
@edmorley edmorley mentioned this pull request Feb 28, 2022
Merged
@edmorley
Copy link

edmorley commented Mar 5, 2022

@softprops Hi! I don't suppose you know when you might have a spare moment to look at #58 / #57 so a new version of hyperlocal can be published with this change? I'm hoping to use this with bollard to reduce its footprint. Many thanks :-)

edmorley added a commit to edmorley/bollard that referenced this pull request Mar 6, 2022
@edmorley
Only enable the tokio features used by this crate
a948b0c 
Tokio offers the following features:
https://docs.rs/tokio/latest/tokio/#feature-flags

This removes the following unused Tokio features from `dependencies`:
- `fs`: Since it's only used by examples/tests/doctests:
   https://github.com/fussybeaver/bollard/search?q=%22tokio%3A%3Afs%22
- `rt` / `rt-multi-thread`: Since it's not used by bollard itself, and should be
   specified by end-users in their application. By activating `rt-multi-thread`
   by default, it means anyone not using the multi-thread runtime has to pay
   the dependency/compile time price for it regardless.

And the following from `dev-dependencies`:
- `time` / `net`: Since they are already specified in `dependencies`, and it's
   not necessary to duplicate them in `dev-dependencies` as Cargo will perform
   feature unification.
- `rt`: Since `rt-multi-thread` includes `rt` already.
- `io-std`: Since it's not used by any examples/tests and isn't even one
   of the official top-level features anyway:
   https://docs.rs/tokio/latest/tokio/#feature-flags

Note: The full benefits of this change won't be realised until this `hyperlocal`
change is picked up in the next `hyperlocal` release:
softprops/hyperlocal#54
@edmorley edmorley mentioned this pull request Mar 6, 2022
Merged
edmorley added a commit to edmorley/bollard that referenced this pull request Mar 8, 2022
@edmorley
Only enable the tokio features used by this crate
2adefd2 
Tokio offers the following features:
https://docs.rs/tokio/latest/tokio/#feature-flags

This removes the following unused Tokio features from `dependencies`:
- `fs`: Since it's only used by examples/tests/doctests:
   https://github.com/fussybeaver/bollard/search?q=%22tokio%3A%3Afs%22
- `rt` / `rt-multi-thread`: Since it's not used by bollard itself, and should be
   specified by end-users in their application. By activating `rt-multi-thread`
   by default, it means anyone not using the multi-thread runtime has to pay
   the dependency/compile time price for it regardless.

And the following from `dev-dependencies`:
- `time` / `net`: Since they are already specified in `dependencies`, and it's
   not necessary to duplicate them in `dev-dependencies` as Cargo will perform
   feature unification.
- `rt`: Since `rt-multi-thread` includes `rt` already.
- `io-std`: Since it's not used by any examples/tests and isn't even one
   of the official top-level features anyway:
   https://docs.rs/tokio/latest/tokio/#feature-flags

Note: The full benefits of this change won't be realised until this `hyperlocal`
change is picked up in the next `hyperlocal` release:
softprops/hyperlocal#54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@onalante-msft @softprops @edmorley