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

RFC: Increase buffer size #93

Merged
merged 3 commits into from Nov 9, 2022
Merged

RFC: Increase buffer size #93

merged 3 commits into from Nov 9, 2022

Conversation

cgzones
Copy link
Contributor

@cgzones cgzones commented Nov 9, 2022

Tested against cov-analysis-linux64-2022.06.tar.gz (1.2G) with /tmp mounted on a tmpfs:

$ hyperfine -w 1 -m 10 -c 'rm -Rf /tmp/benchtest/*' './target/release/examples/uncompress_tool uncompress-archive cov-analysis-linux64-2022.06.tar.gz /tmp/benchtest true'
Benchmark 1: ./target/release/examples/uncompress_tool uncompress-archive cov-analysis-linux64-2022.06.tar.gz /tmp/benchtest true
  Time (mean ± σ):      9.516 s ±  0.053 s    [User: 7.088 s, System: 2.401 s]
  Range (min … max):    9.462 s …  9.626 s    10 runs

$ hyperfine -w 1 -m 10 -c 'rm -Rf /tmp/benchtest/*' './target/release/examples/uncompress_tool uncompress-archive cov-analysis-linux64-2022.06.tar.gz /tmp/benchtest true'
Benchmark 1: ./target/release/examples/uncompress_tool uncompress-archive cov-analysis-linux64-2022.06.tar.gz /tmp/benchtest true
  Time (mean ± σ):      8.055 s ±  0.027 s    [User: 6.386 s, System: 1.643 s]
  Range (min … max):    8.008 s …  8.085 s    10 runs

@coveralls
Copy link

coveralls commented Nov 9, 2022

Pull Request Test Coverage Report for Build 3429684561

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 3 unchanged lines in 2 files lost coverage.
  • Overall coverage decreased (-0.8%) to 85.57%

Files with Coverage Reduction New Missed Lines %
../../../.cargo/registry/src/github.com-1ecc6299db9ec823/futures-util-0.3.25/src/sink/feed.rs 1 94.44%
../../../.cargo/registry/src/github.com-1ecc6299db9ec823/futures-channel-0.3.25/src/mpsc/mod.rs 2 90.41%
Totals Coverage Status
Change from base Build 3331708289: -0.8%
Covered Lines: 338
Relevant Lines: 395

💛 - Coveralls

@otavio
Copy link
Member

otavio commented Nov 9, 2022

Did you compare the memory use? This might impact embedded devices, hence my question.

@cgzones
Copy link
Contributor Author

cgzones commented Nov 9, 2022

time(1) shows Maximum resident set size (kbytes) of 7852 for both.
heaptrack(1) shows peak RSS (including heaptrack overhead) of 10.79M for both.

@otavio
Copy link
Member

otavio commented Nov 9, 2022

Can you add an entry on CHANGES file before merging?

Tested against cov-analysis-linux64-2022.06.tar.gz (1.2G) with /tmp
mounted on a tmpfs:

    $ hyperfine -w 1 -m 10 -c 'rm -Rf /tmp/benchtest/*' './target/release/examples/uncompress_tool uncompress-archive cov-analysis-linux64-2022.06.tar.gz /tmp/benchtest true'
    Benchmark 1: ./target/release/examples/uncompress_tool uncompress-archive cov-analysis-linux64-2022.06.tar.gz /tmp/benchtest true
      Time (mean ± σ):      9.516 s ±  0.053 s    [User: 7.088 s, System: 2.401 s]
      Range (min … max):    9.462 s …  9.626 s    10 runs

    $ hyperfine -w 1 -m 10 -c 'rm -Rf /tmp/benchtest/*' './target/release/examples/uncompress_tool uncompress-archive cov-analysis-linux64-2022.06.tar.gz /tmp/benchtest true'
    Benchmark 1: ./target/release/examples/uncompress_tool uncompress-archive cov-analysis-linux64-2022.06.tar.gz /tmp/benchtest true
      Time (mean ± σ):      8.055 s ±  0.027 s    [User: 6.386 s, System: 1.643 s]
      Range (min … max):    8.008 s …  8.085 s    10 runs
@otavio
Copy link
Member

otavio commented Nov 9, 2022

We might need to raise the MSRV due to Darwin. Are you aware of any possible cause of this error?

@otavio otavio merged commit d081cf2 into OSSystems:master Nov 9, 2022
@cgzones cgzones deleted the buffersize branch November 9, 2022 21:08
cgzones added a commit to cgzones/compress-tools-rs that referenced this pull request Nov 10, 2022
otavio pushed a commit that referenced this pull request Nov 10, 2022
otavio added a commit that referenced this pull request Jan 9, 2023
Signed-off-by: Otavio Salvador <otavio@ossystems.com.br>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants