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

Performance issue after upgrading to 1.3.0 #138

Closed
Veritaris opened this issue May 19, 2024 · 8 comments
Closed

Performance issue after upgrading to 1.3.0 #138

Veritaris opened this issue May 19, 2024 · 8 comments
Labels
bug Something isn't working fix merged Fix is merged, but the issue will stay open until it's released.

Comments

@Veritaris
Copy link

Describe the bug
After updating from old-one 0.6.6 version to latest 1.3.0 I mentioned tragical compression / writing slow

To Reproduce
Use the following code snippet:

use std::fs;
use std::fs::File;
use std::io::{BufReader, BufWriter, stdout, Write};
use std::path::Path;

use clap::Parser;
use linked_hash_map::LinkedHashMap;
// uncomment next lint for 1.3.0, comment for 0.6.6
use zip::write::{SimpleFileOptions};
// uncomment next lint for 0.6.6, comment for 1.3.0
//use zip::write::{FileOptions};
use zip::{CompressionMethod, ZipArchive};

fn main() {
    let limit = 1024;
    let output_path = std::path::Path::new("output.jar");
    let output_file = std::fs::File::create(output_path).unwrap();
    let output_buffer = std::io::BufWriter::with_capacity(8 * 1024 * 1024, output_file);
    let mut output_jar = zip::ZipWriter::new(output_buffer);
    let data = &[0u8; 8 * 1024]; // 1 Kb
    for i in 0..limit {
        // use next line for 1.3.0
        output_jar.start_file(format!("{}.txt", i), SimpleFileOptions::default().compression_method(CompressionMethod::Deflated)).unwrap();
        // use next line for 0.6.6
        // output_jar.start_file(format!("{}.txt", i), FileOptions::default().compression_method(CompressionMethod::Deflated)).unwrap();
        output_jar.write(data).unwrap();
    }
}

And in Cargo.toml

zip = { version = "1.3.0" }
// or 
zip = { version = "0.6.6" }

On my machine I got the following result:

Deflate, zip version 1.3.0
(master) veritaris@veras reBOrN 🐒 cargo build --release && time ./target/release/reBOrN

real    0m45.777s
user    0m45.067s
sys     0m0.037s

Deflate, zip version 0.6.6
(master) veritaris@veras reBOrN 🐒 cargo build --release && time ./target/release/reBOrN

real    0m0.230s
user    0m0.030s
sys     0m0.011s

I also measured time for Stored option when using 1.3.0:

Stored, zip version 1.3.0
(master) veritaris@veras reBOrN 🐒 cargo build --release && time ./target/release/reBOrN

real    0m0.341s
user    0m0.011s
sys     0m0.015s

Expected behavior
Write speed should be kind of equal for both library version

Screenshots

Desktop (please complete the following information):

  • OS: macOS Sonoma 14.3

Additional context
Add any other context about the problem here.

@Veritaris Veritaris added the bug Something isn't working label May 19, 2024
@Pr0methean
Copy link
Member

It's likely that #93 will fix this, and I believe it has a better than 50/50 chance of being merged by the end of May 2024.

@Pr0methean Pr0methean added the help wanted Extra attention is needed label May 20, 2024
@Pr0methean
Copy link
Member

I'm tagging this bug with "help wanted" in case #93 is unexpectedly delayed and someone finds a simpler optimization in the meantime.

@Shatur
Copy link

Shatur commented May 20, 2024

I noticed huge performance regration too, but looks like it caused by flate2 dependency. I use zip 0.6.6 and when I update flate2 from 1.0.27 to 1.0.28 - I have the mentioned problem.

@Veritaris
Copy link
Author

Veritaris commented May 20, 2024

Hi!
Cool, thanks for the answer
Moreover, I dig into the source code and found why default behavior changed to slow. In 0.6.6 flate2 is used with a simple compression algorithm, but in 1.3.0 deflate-zopfli feature is used by default, and if compression_level is None thus it's 24 and used higher compression rate over compression speed 😅
So to speed up with still keeping compression can be achieved with setting .compression_level(Some(X)) where X i64 in range 0..9
Thus nvm, there's no bug in the code for my problem, just changed default behavior that is expected when major version is changed :D

@Shatur
Copy link

Shatur commented May 20, 2024

But I use 0.6.6, so I'm not sure if my problem related... I will try updating zip and write you back.

@Shatur
Copy link

Shatur commented May 20, 2024

Ah, I probably need a new release with the deflate-zopfli change.

@Pr0methean Pr0methean removed the help wanted Extra attention is needed label May 21, 2024
@Pr0methean
Copy link
Member

The changes in zip-rs:zip2:897cca50b3ce220f0c70aea25e60764ec72fd066...zip-rs:zip2:20fa9edd14c8f53a56888523646ada42d7437f2c should fix this once they're released.

@Pr0methean Pr0methean added the fix merged Fix is merged, but the issue will stay open until it's released. label May 21, 2024
@Pr0methean
Copy link
Member

Fix released in 1.3.1.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working fix merged Fix is merged, but the issue will stay open until it's released.
Projects
None yet
Development

No branches or pull requests

3 participants