sile / libflate

A Rust implementation of DEFLATE algorithm and related formats (ZLIB, GZIP)
https://docs.rs/libflate
MIT License
178 stars 35 forks source link

Improve decode code of read_non_compressed_block() #39

Closed Stargateur closed 5 years ago

Stargateur commented 5 years ago

Should improve previous #34 I also try to run hyperfine -m 25 --warmup=3 'target/release/examples/flate -i enwiki-latest-all-titles-in-ns0.gz -o /dev/null gzip-decode' on my VM of arch with a file found here I suppose it's the good one.

I see a little improve from 4.417 +- 0.187 s to 4.193 +- 0.111 s, well I don't really trust this kind of bench on one machine but anyway I was curious.

I don't really know what should be put in the error message in case the read is not complete.

codecov-io commented 5 years ago

Codecov Report

Merging #39 into master will decrease coverage by 0.01%. The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #39      +/-   ##
==========================================
- Coverage   91.36%   91.34%   -0.02%     
==========================================
  Files          14       14              
  Lines        1285     1283       -2     
==========================================
- Hits         1174     1172       -2     
  Misses        111      111
Impacted Files Coverage Δ
src/util.rs 90.9% <0%> (-0.27%) :arrow_down:
src/deflate/encode.rs 96.59% <0%> (-0.03%) :arrow_down:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 2efa0ab...15811cb. Read the comment docs.

Shnatsel commented 5 years ago

I am seeing only a marginal improvement. Before:

  Time (mean ± σ):      2.081 s ±  0.019 s    [User: 2.068 s, System: 0.013 s]
  Range (min … max):    2.055 s …  2.128 s    25 runs

After:

  Time (mean ± σ):      2.050 s ±  0.028 s    [User: 2.038 s, System: 0.011 s]
  Range (min … max):    2.022 s …  2.112 s    25 runs

As measured with hyperfine -m 25 --warmup=3 'target/release/examples/flate -i enwiki-latest-all-titles-in-ns0.gz -o /dev/null gzip-decode'

The good news is that these numbers are stable between runs, so we can rely on them being accurate.

The change looks good to me. The use of take() + read_to_end() is clever!

Stargateur commented 5 years ago

damn didn't have time to fix I forget to run cargo fmt XD

Shnatsel commented 5 years ago

My approval means nothing, I'm just a bystander. We'll need the maintainer to take a look, so you have plenty of time.