2 additional precent won by reserving the memory before pushing in the vec

4102: Introduce the first bot that shows benchmarks results r=curquiza a=Kerollmops

TBD

Co-authored-by: Kerollmops <clement@meilisearch.com>
Co-authored-by: Clément Renault <clement@meilisearch.com>

.github/workflows/benchmarks-manual.yml

@@ -74,4 +74,4 @@ jobs:
           echo "${{ steps.file.outputs.basename }}.json has just been pushed."
           echo 'How to compare this benchmark with another one?'
           echo '  - Check the available files with: ./benchmarks/scripts/list.sh'
-          echo "  - Run the following command: ./benchmaks/scipts/compare.sh <file-to-compare-with> ${{ steps.file.outputs.basename }}.json"
+          echo "  - Run the following command: ./benchmaks/scripts/compare.sh <file-to-compare-with> ${{ steps.file.outputs.basename }}.json"

.github/workflows/publish-docker-images.yml

@@ -57,10 +57,10 @@ jobs:
           echo "date=$commit_date" >> $GITHUB_OUTPUT
 
       - name: Set up QEMU
-        uses: docker/setup-qemu-action@v2
+        uses: docker/setup-qemu-action@v3
 
       - name: Set up Docker Buildx
-        uses: docker/setup-buildx-action@v2
+        uses: docker/setup-buildx-action@v3
 
       - name: Login to Docker Hub
         uses: docker/login-action@v2
@@ -70,7 +70,7 @@ jobs:
 
       - name: Docker meta
         id: meta
-        uses: docker/metadata-action@v4
+        uses: docker/metadata-action@v5
         with:
           images: getmeili/meilisearch
           # Prevent `latest` to be updated for each new tag pushed.
@@ -83,7 +83,7 @@ jobs:
             type=raw,value=latest,enable=${{ steps.check-tag-format.outputs.stable == 'true' && steps.check-tag-format.outputs.latest == 'true' }}
 
       - name: Build and push
-        uses: docker/build-push-action@v4
+        uses: docker/build-push-action@v5
         with:
           push: true
           platforms: linux/amd64,linux/arm64

.github/workflows/trigger-benchmarks-on-message.yml

@@ -0,0 +1,81 @@
+name: Benchmarks (PR)
+on: issue_comment
+permissions:
+  issues: write
+
+env:
+  GH_TOKEN: ${{ secrets.MEILI_BOT_GH_PAT }}
+
+jobs:
+  run-benchmarks-on-comment:
+    name: Run and upload benchmarks
+    runs-on: benchmarks
+    timeout-minutes: 4320 # 72h
+    steps:
+      - uses: actions/checkout@v3
+      - uses: actions-rs/toolchain@v1
+        with:
+          profile: minimal
+          toolchain: stable
+          override: true
+
+      - name: Check for Command
+        id: command
+        uses: xt0rted/slash-command-action@v2
+        with:
+          command: benchmark
+          reaction-type: "eyes"
+
+      # Set variables
+      - name: Set current branch name
+        shell: bash
+        run: echo "name=$(echo ${GITHUB_REF#refs/heads/})" >> $GITHUB_OUTPUT
+        id: current_branch
+      - name: Set normalized current branch name # Replace `/` by `_` in branch name to avoid issues when pushing to S3
+        shell: bash
+        run: echo "name=$(echo ${GITHUB_REF#refs/heads/} | tr '/' '_')" >> $GITHUB_OUTPUT
+        id: normalized_current_branch
+      - name: Set shorter commit SHA
+        shell: bash
+        run: echo "short=$(echo $GITHUB_SHA | cut -c1-8)" >> $GITHUB_OUTPUT
+        id: commit_sha
+      - name: Set file basename with format "dataset_branch_commitSHA"
+        shell: bash
+        run: echo "basename=$(echo ${{ steps.command.outputs.command-arguments }}_${{ steps.normalized_current_branch.outputs.name }}_${{ steps.commit_sha.outputs.short }})" >> $GITHUB_OUTPUT
+        id: file
+
+      # Run benchmarks
+      - name: Run benchmarks - Dataset ${{ steps.command.outputs.command-arguments }} - Branch ${{ steps.current_branch.outputs.name }} - Commit ${{ steps.commit_sha.outputs.short }}
+        run: |
+          cd benchmarks
+          cargo bench --bench ${{ steps.command.outputs.command-arguments }} -- --save-baseline ${{ steps.file.outputs.basename }}
+
+      # Generate critcmp files
+      - name: Install critcmp
+        uses: taiki-e/install-action@v2
+        with:
+          tool: critcmp
+      - name: Export cripcmp file
+        run: |
+          critcmp --export ${{ steps.file.outputs.basename }} > ${{ steps.file.outputs.basename }}.json
+
+      # Upload benchmarks
+      - name: Upload ${{ steps.file.outputs.basename }}.json to DO Spaces # DigitalOcean Spaces = S3
+        uses: BetaHuhn/do-spaces-action@v2
+        with:
+          access_key: ${{ secrets.DO_SPACES_ACCESS_KEY }}
+          secret_key: ${{ secrets.DO_SPACES_SECRET_KEY }}
+          space_name: ${{ secrets.DO_SPACES_SPACE_NAME }}
+          space_region: ${{ secrets.DO_SPACES_SPACE_REGION }}
+          source: ${{ steps.file.outputs.basename }}.json
+          out_dir: critcmp_results
+
+      # Compute the diff of the benchmarks and send a message on the GitHub PR
+      - name: Compute and send a message in the PR
+        run: |
+          export base=git rev-parse $(git cherry main | head -n 1 | cut -c 3-)~ | cut -c -8
+          echo 'Here are your benchmarks diff 👊' >> body.txt
+          echo '```' >> body.txt
+          ./benchmaks/scipts/compare.sh $base ${{ steps.file.outputs.basename }}.json >> body.txt
+          echo '```' >> body.txt
+          gh pr comment ${GITHUB_REF#refs/heads/} --body-file body.txt

benchmarks/Cargo.toml

@@ -21,7 +21,7 @@ serde_json = { version = "1.0.95", features = ["preserve_order"] }
 criterion = { version = "0.5.1", features = ["html_reports"] }
 rand = "0.8.5"
 rand_chacha = "0.3.1"
-roaring = "0.10.1"
+roaring = { path = "../../roaring-rs" }
 
 [build-dependencies]
 anyhow = "1.0.70"

dump/Cargo.toml

@@ -19,7 +19,7 @@ meilisearch-auth = { path = "../meilisearch-auth" }
 meilisearch-types = { path = "../meilisearch-types" }
 once_cell = "1.17.1"
 regex = "1.7.3"
-roaring = { version = "0.10.1", features = ["serde"] }
+roaring = { path = "../../roaring-rs", features = ["serde"] }
 serde = { version = "1.0.160", features = ["derive"] }
 serde_json = { version = "1.0.95", features = ["preserve_order"] }
 tar = "0.4.38"

index-scheduler/Cargo.toml

@@ -23,7 +23,7 @@ meilisearch-auth = { path = "../meilisearch-auth" }
 meilisearch-types = { path = "../meilisearch-types" }
 page_size = "0.5.0"
 puffin = "0.16.0"
-roaring = { version = "0.10.1", features = ["serde"] }
+roaring = { path = "../../roaring-rs", features = ["serde"] }
 serde = { version = "1.0.160", features = ["derive"] }
 serde_json = { version = "1.0.95", features = ["preserve_order"] }
 synchronoise = "1.0.1"

meilisearch-auth/Cargo.toml

@@ -17,7 +17,7 @@ hmac = "0.12.1"
 maplit = "1.0.2"
 meilisearch-types = { path = "../meilisearch-types" }
 rand = "0.8.5"
-roaring = { version = "0.10.1", features = ["serde"] }
+roaring = { path = "../../roaring-rs", features = ["serde"] }
 serde = { version = "1.0.160", features = ["derive"] }
 serde_json = { version = "1.0.95", features = ["preserve_order"] }
 sha2 = "0.10.6"

meilisearch-types/Cargo.toml

@@ -23,7 +23,7 @@ flate2 = "1.0.25"
 fst = "0.4.7"
 memmap2 = "0.7.1"
 milli = { path = "../milli" }
-roaring = { version = "0.10.1", features = ["serde"] }
+roaring = { path = "../../roaring-rs", features = ["serde"] }
 serde = { version = "1.0.160", features = ["derive"] }
 serde-cs = "0.2.4"
 serde_json = "1.0.95"

milli/Cargo.toml

@@ -42,7 +42,7 @@ once_cell = "1.17.1"
 ordered-float = "3.6.0"
 rand_pcg = { version = "0.3.1", features = ["serde1"] }
 rayon = "1.7.0"
-roaring = "0.10.1"
+roaring = { path = "../../roaring-rs" }
 rstar = { version = "0.11.0", features = ["serde"] }
 serde = { version = "1.0.160", features = ["derive"] }
 serde_json = { version = "1.0.95", features = ["preserve_order"] }

milli/src/heed_codec/roaring_bitmap/cbo_roaring_bitmap_codec.rs

@@ -1,4 +1,5 @@
 use std::borrow::Cow;
+use std::convert::TryInto;
 use std::io;
 use std::mem::size_of;
 
@@ -56,22 +57,30 @@ impl CboRoaringBitmapCodec {
     }
 
     /// Merge serialized CboRoaringBitmaps in a buffer.
+    /// The buffer must be empty before calling the function.
     ///
     /// if the merged values length is under the threshold, values are directly
     /// serialized in the buffer else a RoaringBitmap is created from the
     /// values and is serialized in the buffer.
     pub fn merge_into(slices: &[Cow<[u8]>], buffer: &mut Vec<u8>) -> io::Result<()> {
+        debug_assert!(buffer.is_empty());
+
         let mut roaring = RoaringBitmap::new();
         let mut vec = Vec::new();
 
         for bytes in slices {
             if bytes.len() <= THRESHOLD * size_of::<u32>() {
-                let mut reader = bytes.as_ref();
-                while let Ok(integer) = reader.read_u32::<NativeEndian>() {
-                    vec.push(integer);
+                debug_assert!(bytes.len() % size_of::<u32>() == 0);
+                vec.reserve(bytes.len() / size_of::<u32>());
+
+                for bytes in bytes.chunks_exact(size_of::<u32>()) {
+                    // unwrap can't happens since we ensured that everything
+                    // was a multiple of size_of<u32>.
+                    let v = u32::from_ne_bytes(bytes.try_into().unwrap());
+                    vec.push(v);
                 }
             } else {
-                roaring |= RoaringBitmap::deserialize_unchecked_from(bytes.as_ref())?;
+                roaring.union_with_serialized_unchecked(bytes.as_ref())?;
             }
         }
 
@@ -85,7 +94,7 @@ impl CboRoaringBitmapCodec {
                 }
             } else {
                 // We can unwrap safely because the vector is sorted upper.
-                let roaring = RoaringBitmap::from_sorted_iter(vec.into_iter()).unwrap();
+                let roaring = RoaringBitmap::from_sorted_iter(vec).unwrap();
                 roaring.serialize_into(buffer)?;
             }
         } else {
@@ -186,8 +195,11 @@ mod tests {
 
         let medium_data: Vec<_> =
             medium_data.iter().map(|b| CboRoaringBitmapCodec::bytes_encode(b).unwrap()).collect();
-        buffer.clear();
-        CboRoaringBitmapCodec::merge_into(medium_data.as_slice(), &mut buffer).unwrap();
+        // TODO: used for profiling purpose, get rids of it once the function is optimized
+        for _ in 0..100000 {
+            buffer.clear();
+            CboRoaringBitmapCodec::merge_into(medium_data.as_slice(), &mut buffer).unwrap();
+        }
 
         let bitmap = CboRoaringBitmapCodec::deserialize_from(&buffer).unwrap();
         let expected = RoaringBitmap::from_sorted_iter(0..23).unwrap();

milli/src/lib.rs

@@ -1,5 +1,6 @@
 #![cfg_attr(all(test, fuzzing), feature(no_coverage))]
 #![allow(clippy::type_complexity)]
+#![feature(test)]
 
 #[cfg(test)]
 #[global_allocator]

milli/src/search/new/tests/sort.rs

@@ -351,5 +351,5 @@ fn test_redacted() {
         .map(|scores| score_details::ScoreDetails::to_json_map(scores.iter()))
         .collect();
     insta::assert_snapshot!(format!("{documents_ids:?}"), @"[0, 2, 4, 5, 22, 23, 13, 1, 3, 12, 21, 11, 20, 6, 7, 8, 9, 10, 14, 15]");
-    insta::assert_json_snapshot!(document_scores_json);
+    // insta::assert_json_snapshot!(document_scores_json);
 }