Criterion benchmarks

[im7mortal]

#723

Draft of pull request. Check CIs.

• The PR title must conform to Conventional Commits and start with one of the types specified by the Angular convention.

[gemini-code-assist]

Code Review

This pull request introduces comprehensive Criterion-based benchmarks for zip archive operations, including reading, writing, and merging. It adds the criterion and tempfile dev-dependencies and a new benchmark suite in benches/criterion_bench.rs that supports low-memory environments. Feedback was provided to ensure dependency versions in Cargo.toml follow the project's convention of using carets when patch versions are specified.

[im7mortal]

Hi @Its-Just-Nans
I see that fail reason is hardened CI

Can you resolve it please? I even didn't touch any CI

AI suggest to edit following step https://github.com/zip-rs/zip2/blob/master/.github/workflows/ci.yaml#L35-L41

[Its-Just-Nans]

Can you resolve it please? I even didn't touch any CI

Criterion cannot compile to big-endian due to half due to zerocopy

• aarch64_be not compiling google/zerocopy#3155
• Not working with aarch64_be VoidStarKat/half-rs#142

[im7mortal]

I see.

Since Criterion is only a dev-dependency, I assume we don’t need to ensure it builds on a variety of platforms.

The AI suggests setting exactly the targets we need: --lib --bins --tests.

cargo +nightly miri test --lib --bins --tests --target aarch64_be-unknown-linux-gnu --no-default-features

[Its-Just-Nans]

The AI suggests setting exactly the targets we need: --lib --bins --tests.

That could be an option but the MR fixing the issue in zerocopy is merged. If we wait for a bump of zerocopy and half, criterion should compile I think

EDIT: criterion now compiles fine

[Its-Just-Nans]

This MR also close #151 I think

[Its-Just-Nans]

@im7mortal

Why did you use fn seeded_random_bytes(size: usize) instead of the getrandom crate?

Also, can we move it to review ?

[im7mortal]

@Its-Just-Nans

1. getrandom is designed so it cannot be deterministic. The function fn seeded_random_bytes(size: usize) always returns the same random sequence. I’ve been wondering whether it’s actually required here. I’ll probably run more tests and then decide. I think I’ll switch back to getrandom later, since the difference is clearly negligible.

2. I will move the PR to review in mid-April. I am currently running tests on it. My local machine is not suitable for benchmarks, as I have many memory and battery tweaks installed (for example, zram). I switched to testing on my Raspberry Pi 3, which I hadn’t used in 10 years. It’s much more stable there, although I have to spend quite a bit of time setting it up.

I still need to decide on a few things. For example, I haven’t decided whether I should send a preallocated buffer or not. On one hand, you could argue that we are not benchmarking the allocator here. On the other hand, if there is code that performs arbitrary writes, the allocator might expose those issues. There are still other aspects I’m checking.

[im7mortal]

Just finished another exhausting round of creating benchmarks.

To validate their usability, I set up a clean environment on a sterile Raspberry Pi 3B.

I decided to keep the test data generator in place.

I’m still not sure which is better: the in-memory reader/writer or the file system reader/writer.

On the graph there MAYBE regressions. I will investigate them to prove that the benchmark serves some purpose.