Merge #213

213: Fix the benchmarks script and names r=Kerollmops a=Kerollmops

The benchmarks compare script was not using the `--output` flag and was therefore failing the download of the JSON reports. We also modified the criterion benchmarks to use shorter names, it helps in looking at the benchmarks in the terminal.

Co-authored-by: Kerollmops <clement@meilisearch.com>

Cargo.lock

@@ -900,7 +900,7 @@ dependencies = [
 
 [[package]]
 name = "helpers"
-version = "0.2.1"
+version = "0.3.0"
 dependencies = [
  "anyhow",
  "byte-unit",
@@ -954,7 +954,7 @@ dependencies = [
 
 [[package]]
 name = "http-ui"
-version = "0.2.1"
+version = "0.3.0"
 dependencies = [
  "anyhow",
  "askama",
@@ -1095,7 +1095,7 @@ dependencies = [
 
 [[package]]
 name = "infos"
-version = "0.2.1"
+version = "0.3.0"
 dependencies = [
  "anyhow",
  "byte-unit",
@@ -1358,7 +1358,7 @@ dependencies = [
 
 [[package]]
 name = "milli"
-version = "0.2.1"
+version = "0.3.0"
 dependencies = [
  "anyhow",
  "big_s",
@@ -2215,7 +2215,7 @@ dependencies = [
 
 [[package]]
 name = "search"
-version = "0.2.1"
+version = "0.3.0"
 dependencies = [
  "anyhow",
  "byte-unit",

benchmarks/Cargo.toml

@@ -10,7 +10,7 @@ milli = { path = "../milli" }
 
 [dev-dependencies]
 heed = "*" # we want to use the version milli uses
-criterion = "0.3.4"
+criterion = { version = "0.3.4", features = ["html_reports"] }
 
 [build-dependencies]
 anyhow = "1.0"

benchmarks/benches/utils.rs

@@ -1,4 +1,5 @@
 use std::fs::{create_dir_all, remove_dir_all, File};
+use std::path::Path;
 
 use criterion::BenchmarkId;
 use heed::EnvOpenOptions;
@@ -97,7 +98,9 @@ pub fn run_benches(c: &mut criterion::Criterion, confs: &[Conf]) {
     for conf in confs {
         let index = base_setup(conf);
 
-        let mut group = c.benchmark_group(&format!("{}: {}", conf.dataset, conf.group_name));
+        let file_name = Path::new(conf.dataset).file_name().and_then(|f| f.to_str()).unwrap();
+        let name = format!("{}: {}", file_name, conf.group_name);
+        let mut group = c.benchmark_group(&name);
 
         for &query in conf.queries {
             group.bench_with_input(BenchmarkId::from_parameter(query), &query, |b, &query| {
@@ -106,8 +109,7 @@ pub fn run_benches(c: &mut criterion::Criterion, confs: &[Conf]) {
                     let mut search = index.search(&rtxn);
                     search.query(query).optional_words(conf.optional_words);
                     if let Some(filter) = conf.filter {
-                        let filter =
+                        let filter = FilterCondition::from_str(&rtxn, &index, filter).unwrap();
-                            FilterCondition::from_str(&rtxn, &index, filter).unwrap();
                         search.filter(filter);
                     }
                     let _ids = search.execute().unwrap();

benchmarks/scripts/compare.sh

@@ -11,9 +11,9 @@
 # Checking that critcmp is installed
 command -v critcmp > /dev/null 2>&1
 if [[ "$?" -ne 0 ]]; then
-    echo 'You must install critcmp to make this script working.'
+    echo 'You must install critcmp to make this script work.'
-    echo '$ cargo install critcmp'
     echo 'See: https://github.com/BurntSushi/critcmp'
+    echo '  $ cargo install critcmp'
     exit 1
 fi
 
@@ -21,38 +21,30 @@ if [[ $# -ne 2 ]]
   then
     echo 'Need 2 arguments.'
     echo 'Usage: '
-    echo '  $ ./compare.sh file_to_download1 file_to_download2'
+    echo '  $ ./compare.sh old new'
     echo 'Ex:'
     echo '  $ ./compare.sh songs_main_09a4321.json songs_geosearch_24ec456.json'
     exit 1
 fi
 
-file1="$1"
+old_file="$1"
-file2="$2"
+new_file="$2"
 s3_url='https://milli-benchmarks.fra1.digitaloceanspaces.com/critcmp_results'
-file1_s3_url="$s3_url/$file1"
-file2_s3_url="$s3_url/$file2"
-file1_local_path="/tmp/$file1"
-file2_local_path="/tmp/$file2"
 
-if [[ ! -f "$file1_local_path" ]]; then
+for file in $old_file $new_file
-    curl "$file1_s3_url" -O "$file1_local_path"
+do
-    if [[ "$?" -ne 0 ]]; then
+    file_s3_url="$s3_url/$file"
-	    echo 'curl command failed.'
+    file_local_path="/tmp/$file"
-	    exit 1
+
+    if [[ ! -f $file_local_path ]]; then
+        curl $file_s3_url --output $file_local_path --silent
+        if [[ "$?" -ne 0 ]]; then
+            echo 'curl command failed.'
+            exit 1
+        fi
     fi
-else
+done
-    echo "$file1 already present in /tmp, no need to download."
-fi
 
-if [[ ! -f "$file2_local_path" ]]; then
+# Print the diff changes between the old and new benchmarks
-    curl "$file2_s3_url" -O "$file2_local_path"
+# by only displaying the lines that have a diff of more than 5%.
-    if [[ "$?" -ne 0 ]]; then
+critcmp --threshold 5 "/tmp/$old_file" "/tmp/$new_file"
-	    echo 'curl command failed.'
-	    exit 1
-    fi
-else
-    echo "$file2 already present in /tmp, no need to download."
-fi
-
-critcmp --color always "$file1_local_path" "$file2_local_path"