ARK-Builders / arklib

Core of the programs in ARK family
MIT License
1 stars 10 forks source link

#65: Add Hash Function Benchmarks #74

Closed tareknaser closed 5 months ago

tareknaser commented 6 months ago

Description

This PR adds benchmarking infrastructure using criterion crate. For now, a single benchmark is added. Related issue:

Changes

The file benches/hash_benchmark.rs includes the compute_bytes_benchmark function, which has three benchmarks for different use cases:

criterion is configured as a dev dependency with the html_reports feature for detailed HTML reports in target/criterion/.

kirillt commented 5 months ago

Looks great. Could you also implement these points?

tareknaser commented 5 months ago

I've set up benchmarks to run automatically on every pull request to the main branch using boa-dev/criterion-compare-action. This configuration runs benchmarks on both the pull request branch and the main branch, and then posts the results as a comment on the pull request. You can see an example of how it looks when merged in the following link: Example

kirillt commented 5 months ago

Can we catch benchmarks errors, too? It's not critical but would be nice.

E.g. I modify the path to something on my filesystem:

[kirill@lenovo arklib]$ git diff benches/index_build_benchmark.rs
diff --git a/benches/index_build_benchmark.rs b/benches/index_build_benchmark.rs
index f3fd97b..8df6959 100644
--- a/benches/index_build_benchmark.rs
+++ b/benches/index_build_benchmark.rs
@@ -2,7 +2,7 @@ use arklib::index::ResourceIndex;
 use criterion::{black_box, criterion_group, criterion_main, Criterion};

 fn index_build_benchmark(c: &mut Criterion) {
-    let path = "tests/"; // Set the path to the directory containing the resources
+    let path = "/mnt/data/tests"; // Set the path to the directory containing the resources

     c.bench_function("index_build", move |b| {
         b.iter(|| {

But it doesn't exist:

[kirill@lenovo arklib]$ ls /mnt/data/tests
ls: cannot access '/mnt/data/tests': No such file or directory

Right now, the benchmark just passes quickly:

[kirill@lenovo arklib]$ RUST_LOG=debug cargo bench index_build
    Finished bench [optimized] target(s) in 0.25s
     Running benches/compute_bytes_benchmark.rs (target/release/deps/compute_bytes_benchmark-1ca61f88413531a5)
Gnuplot not found, using plotters backend
     Running benches/index_build_benchmark.rs (target/release/deps/index_build_benchmark-3021d4b85891b6cc)
Gnuplot not found, using plotters backend
index_build             time:   [33.091 µs 33.677 µs 34.275 µs]
                        change: [-6.2644% -3.3005% -0.2257%] (p = 0.04 < 0.05)
                        Change within noise threshold.
Found 7 outliers among 100 measurements (7.00%)
  3 (3.00%) low mild
  4 (4.00%) high mild

It's kinda expected, but if we can catch this error, it would be great.

kirillt commented 5 months ago

Couple more actions required:

kirillt commented 5 months ago

Also solves: