Majored / rs-async-zip

An asynchronous ZIP archive reading/writing crate.
MIT License
131 stars 44 forks source link

Port from futures-util to futures-lite #112

Closed notgull closed 9 months ago

notgull commented 11 months ago

futures-lite is a subset of futures-util that is smaller and compiles much faster.

notgull commented 11 months ago

@Majored Any chance you can review this PR?

Majored commented 10 months ago

futures-lite advertises itself as much faster to compile than the full futures crate, but after a quick test locally, it doesn't see the same performance benefits over futures-util.

I'd rather stick to futures-util given it's actually part of the full crate.

notgull commented 10 months ago

Current master:

$ cargo clean && time cargo build -j 1 -q
cargo build -j 1 -q  10.84s user 1.81s system 95% cpu 13.251 total

This branch:

$ cargo clean && time cargo build -j 1 -q
cargo build -j 1 -q  8.76s user 1.35s system 96% cpu 10.508 total

After an adjustment to remove some dependencies:

$ cargo clean && time cargo build -j 1 -q
cargo build -j 1 -q  8.59s user 1.31s system 99% cpu 9.931 tota

I don't think that a 20% reduction in build times is anything to sneeze at here. Although you mention "performance", is there a benchmark that you're using here

I'd rather stick to futures-util given it's actually part of the full crate.

Could you elaborate here?

Majored commented 10 months ago

Although you mention "performance", is there a benchmark that you're using here

Ah, bad wording on my part - I was still referring to the compile speeds.

Could you elaborate here?

I essentially just don't want to be chasing minor improvements in compile speeds by constantly switching out different dependencies and ones that may be more obscure/less maintained than others. Whilst I appreciate that futures-lite is part of the smol ecosystem, futures-util is more than suitable currently I feel - we're only pulling in what we need.

notgull commented 10 months ago

I essentially just don't want to be chasing minor improvements

I would argue 20% is not just a minor improvement.

ones that may be more obscure/less maintained than others

The maintainer of futures-util is also a maintainer of futures-lite. So maintainership shouldn't be an issue.

we're only pulling in what we need.

By pulling in futures-util, you are also pulling in:

So I think that the subset of futures-util this crate uses is serviced better by futures-lite.