Merge branch 'main' into dependabot

.github/workflows/canbench-post-comment.yml

@@ -0,0 +1,38 @@
+name: Post Canbench results
+
+on:
+  workflow_run:
+    workflows: ["CI"]
+    types:
+      - completed
+
+jobs:
+  download-results:
+    runs-on: ubuntu-latest
+    outputs:
+      matrix: ${{ steps.set-benchmarks.outputs.matrix }}
+      pr_number: ${{ steps.set-benchmarks.outputs.pr_number }}
+    steps:
+      - uses: actions/checkout@v4
+
+      - uses: dawidd6/action-download-artifact@09f2f74827fd3a8607589e5ad7f9398816f540fe
+        with:
+          run_id: ${{ github.event.workflow_run.id }}
+
+      - id: set-benchmarks
+        run: bash ./scripts/ci_download_canbench_artifacts.sh
+
+  post-comment:
+    runs-on: ubuntu-latest
+    needs: [download-results]
+    strategy:
+      matrix: ${{fromJSON(needs.download-results.outputs.matrix)}}
+    steps:
+      - name: Post comment
+        uses: thollander/actions-comment-pull-request@v2
+        with:
+          message: |
+            ${{ matrix.benchmark.result }}
+          comment_tag: ${{ matrix.benchmark.title }}
+          pr_number: ${{ needs.download-results.outputs.pr_number }}
+

.github/workflows/ci.yml

@@ -76,11 +76,6 @@ jobs:
   benchmark:
     runs-on: ubuntu-latest
     needs: build
-    permissions:
-      # To allow writing comments to the PR in forked repositorites.
-      # See https://github.com/thollander/actions-comment-pull-request/issues/181
-      pull-requests: write
-
     env:
       PROJECT_DIR: .
     steps:
@@ -93,16 +88,13 @@ jobs:
           ref: main
           path: _canbench_main_branch
 
-      - name: Cache Cargo
-        uses: actions/cache@v2
+      - uses: actions/cache@v2
         with:
           path: |
             ~/.cargo/registry
             ~/.cargo/git
             target
-          key: ${{ matrix.build }}-cargo-${{ hashFiles('**/Cargo.lock') }}
-          restore-keys: |
-            ${{ matrix.build }}-cargo-
+          key: ${{ runner.os }}-cargo-${{ hashFiles('**/Cargo.lock') }}-1
 
       - name: Install Rust
         run: |
@@ -112,17 +104,25 @@ jobs:
 
       - name: Benchmark
         run: |
-          bash .github/workflows/canbench_ci_run_benchmark.sh $PROJECT_DIR
+          bash ./scripts/ci_run_benchmark.sh $PROJECT_DIR ${{ github.job }}
+
+      - uses: actions/upload-artifact@v4
+        with:
+          name: canbench_result_${{github.job}}
+          path: /tmp/canbench_result_${{ github.job }}
+
+      - name: Save PR number
+        run: |
+          echo ${{ github.event.number }} > /tmp/pr_number
 
-      - name: Post comment
-        uses: thollander/actions-comment-pull-request@v2
+      - uses: actions/upload-artifact@v4
         with:
-          filePath: /tmp/canbench_comment_message.txt
-          comment_tag: ${{ env.PROJECT_DIR }}
+          name: pr_number
+          path: /tmp/pr_number
 
       - name: Pass or fail
         run: |
-          bash .github/workflows/canbench_ci_post_run_benchmark.sh
+          bash ./scripts/ci_post_run_benchmark.sh
 
   checks-pass:
     # Always run this job!

scripts/ci_download_canbench_artifacts.sh

@@ -0,0 +1,29 @@
+#!/usr/bin/env bash
+set -Eexuo pipefail
+
+# Identifies the benchmarks provided in the artifacts and outputs them.
+
+json_array="["
+# Loop through each file with prefix "canbench_result_" in the current directory
+for file in canbench_result_*; do
+if [ -e "$file" ]; then  # Check if the file exists.
+  # Read the content of the file, escaping double quotes and adding escaped newlines
+  content=$(<"$file/$file" sed 's/"/\\"/g' | awk '{printf "%s\\n", $0}' | sed '$ s/\\n$//')
+
+  # Construct a JSON object for the current file with "title" and "result" keys
+  json_object="{\"title\":\"$file\",\"result\":\"$content\"},"
+
+  # Append the JSON object to the array string
+  json_array+="$json_object"
+fi
+done
+
+# Remove the trailing comma from the JSON array string
+json_array=${json_array%,}
+
+# Close the JSON array string
+json_array+="]"
+
+# Output the benchmarks and PR number to be used by the next job.
+echo "matrix={\"benchmark\": $json_array}" >> "$GITHUB_OUTPUT"
+echo "pr_number=$(cat ./pr_number/pr_number)" >> "$GITHUB_OUTPUT"

.github/workflows/canbench_ci_run_benchmark.sh → scripts/ci_run_benchmark.sh

@@ -7,8 +7,11 @@ set -Eexuo pipefail
 # Path to run `canbench` from.
 CANISTER_PATH=$1
 
+# The name of the job in CI
+CANBENCH_JOB_NAME=$2
+
 # Must match the file specified in the github action.
-COMMENT_MESSAGE_PATH=/tmp/canbench_comment_message.txt
+COMMENT_MESSAGE_PATH=/tmp/canbench_result_${CANBENCH_JOB_NAME}
 
 # Github CI is expected to have the main branch checked out in this folder.
 MAIN_BRANCH_DIR=_canbench_main_branch
@@ -45,7 +48,7 @@ fi
 popd
 
 
-echo "# \`canbench\` 🏋" > $COMMENT_MESSAGE_PATH
+echo "# \`canbench\` 🏋 (dir: $CANISTER_PATH)" > "$COMMENT_MESSAGE_PATH"
 
 # Detect if there are performance changes relative to the main branch.
 if [ -f "$MAIN_BRANCH_RESULTS_FILE" ]; then
@@ -54,23 +57,15 @@ if [ -f "$MAIN_BRANCH_RESULTS_FILE" ]; then
 
   # Run canbench to compare result to main branch.
   pushd "$CANISTER_PATH"
-  set +e
-  canbench --less-verbose > $CANBENCH_OUTPUT
-  RES=$?
-  set -e
+  canbench --less-verbose > "$CANBENCH_OUTPUT"
   popd
 
-  if [ "$RES" -eq 0 ]; then
-    if grep -q "(regress\|(improved by" "${CANBENCH_OUTPUT}"; then
-      echo "**Significant performance change detected! ⚠️**
-      " >> $COMMENT_MESSAGE_PATH;
-    else
-      echo "**No significant performance changes detected ✅**
-      " >> $COMMENT_MESSAGE_PATH
-    fi
+  if grep -q "(regress\|(improved by" "${CANBENCH_OUTPUT}"; then
+    echo "**Significant performance change detected! ⚠️**
+    " >> "$COMMENT_MESSAGE_PATH"
   else
-    echo "Failed to run \`canbench\` against main branch ⚠️
-    " >> $COMMENT_MESSAGE_PATH
+    echo "**No significant performance changes detected ✅**
+    " >> "$COMMENT_MESSAGE_PATH"
   fi
 fi
 
@@ -81,7 +76,7 @@ fi
   echo "\`\`\`"
   cat "$CANBENCH_OUTPUT"
   echo "\`\`\`"
-} >> $COMMENT_MESSAGE_PATH
+} >> "$COMMENT_MESSAGE_PATH"
 
 # Output the comment to stdout.
-cat $COMMENT_MESSAGE_PATH
+cat "$COMMENT_MESSAGE_PATH"

src/base_vec.rs

@@ -31,7 +31,7 @@
 //! type is fixed in size, the `SLOT_SIZE` is equal to the max size.
 //! Otherwise, the `SLOT_SIZE` is the max size plus the number of
 //! bytes required to represent integers up to that max size.
-use crate::storable::{bounds, bytes_to_store_size};
+use crate::storable::{bounds, bytes_to_store_size_bounded};
 use crate::{
     read_u32, read_u64, safe_write, write_u32, write_u64, Address, GrowFailed, Memory, Storable,
 };
@@ -341,7 +341,7 @@ impl<T: Storable + fmt::Debug, M: Memory> fmt::Debug for BaseVec<T, M> {
 
 fn slot_size<T: Storable>() -> u32 {
     let t_bounds = bounds::<T>();
-    t_bounds.max_size + bytes_to_store_size(&t_bounds)
+    t_bounds.max_size + bytes_to_store_size_bounded(&t_bounds)
 }
 
 pub struct Iter<'a, T, M>

src/storable.rs

@@ -4,6 +4,8 @@ use std::cmp::{Ordering, Reverse};
 use std::convert::{TryFrom, TryInto};
 use std::fmt;
 
+mod tuples;
+
 #[cfg(test)]
 mod tests;
 
@@ -50,6 +52,7 @@ pub trait Storable {
     }
 }
 
+#[derive(Debug, PartialEq)]
 /// States whether the type's size is bounded or unbounded.
 pub enum Bound {
     /// The type has no size bounds.
@@ -395,103 +398,6 @@ impl<T: Storable> Storable for Reverse<T> {
     const BOUND: Bound = T::BOUND;
 }
 
-impl<A, B> Storable for (A, B)
-where
-    A: Storable,
-    B: Storable,
-{
-    fn to_bytes(&self) -> Cow<[u8]> {
-        match Self::BOUND {
-            Bound::Bounded { max_size, .. } => {
-                let mut bytes = vec![0; max_size as usize];
-                let a_bytes = self.0.to_bytes();
-                let b_bytes = self.1.to_bytes();
-
-                let a_bounds = bounds::<A>();
-                let b_bounds = bounds::<B>();
-
-                let a_max_size = a_bounds.max_size as usize;
-                let b_max_size = b_bounds.max_size as usize;
-
-                debug_assert!(a_bytes.len() <= a_max_size);
-                debug_assert!(b_bytes.len() <= b_max_size);
-
-                bytes[0..a_bytes.len()].copy_from_slice(a_bytes.borrow());
-                bytes[a_max_size..a_max_size + b_bytes.len()].copy_from_slice(b_bytes.borrow());
-
-                let a_size_len = bytes_to_store_size(&a_bounds) as usize;
-                let b_size_len = bytes_to_store_size(&b_bounds) as usize;
-
-                let sizes_offset: usize = a_max_size + b_max_size;
-
-                encode_size(
-                    &mut bytes[sizes_offset..sizes_offset + a_size_len],
-                    a_bytes.len(),
-                    &a_bounds,
-                );
-                encode_size(
-                    &mut bytes[sizes_offset + a_size_len..sizes_offset + a_size_len + b_size_len],
-                    b_bytes.len(),
-                    &b_bounds,
-                );
-
-                Cow::Owned(bytes)
-            }
-            _ => todo!("Serializing tuples with unbounded types is not yet supported."),
-        }
-    }
-
-    fn from_bytes(bytes: Cow<[u8]>) -> Self {
-        match Self::BOUND {
-            Bound::Bounded { max_size, .. } => {
-                assert_eq!(bytes.len(), max_size as usize);
-
-                let a_bounds = bounds::<A>();
-                let b_bounds = bounds::<B>();
-                let a_max_size = a_bounds.max_size as usize;
-                let b_max_size = b_bounds.max_size as usize;
-                let sizes_offset = a_max_size + b_max_size;
-
-                let a_size_len = bytes_to_store_size(&a_bounds) as usize;
-                let b_size_len = bytes_to_store_size(&b_bounds) as usize;
-                let a_len = decode_size(&bytes[sizes_offset..sizes_offset + a_size_len], &a_bounds);
-                let b_len = decode_size(
-                    &bytes[sizes_offset + a_size_len..sizes_offset + a_size_len + b_size_len],
-                    &b_bounds,
-                );
-
-                let a = A::from_bytes(Cow::Borrowed(&bytes[0..a_len]));
-                let b = B::from_bytes(Cow::Borrowed(&bytes[a_max_size..a_max_size + b_len]));
-
-                (a, b)
-            }
-            _ => todo!("Deserializing tuples with unbounded types is not yet supported."),
-        }
-    }
-
-    const BOUND: Bound = {
-        match (A::BOUND, B::BOUND) {
-            (Bound::Bounded { .. }, Bound::Bounded { .. }) => {
-                let a_bounds = bounds::<A>();
-                let b_bounds = bounds::<B>();
-
-                let max_size = a_bounds.max_size
-                    + b_bounds.max_size
-                    + bytes_to_store_size(&a_bounds)
-                    + bytes_to_store_size(&b_bounds);
-
-                let is_fixed_size = a_bounds.is_fixed_size && b_bounds.is_fixed_size;
-
-                Bound::Bounded {
-                    max_size,
-                    is_fixed_size,
-                }
-            }
-            _ => Bound::Unbounded,
-        }
-    };
-}
-
 impl<T: Storable> Storable for Option<T> {
     fn to_bytes(&self) -> Cow<[u8]> {
         match self {
@@ -568,38 +474,18 @@ pub(crate) const fn bounds<A: Storable>() -> Bounds {
     }
 }
 
-fn decode_size(src: &[u8], bounds: &Bounds) -> usize {
-    if bounds.is_fixed_size {
-        bounds.max_size as usize
-    } else if bounds.max_size <= u8::MAX as u32 {
-        src[0] as usize
-    } else if bounds.max_size <= u16::MAX as u32 {
-        u16::from_be_bytes([src[0], src[1]]) as usize
-    } else {
-        u32::from_be_bytes([src[0], src[1], src[2], src[3]]) as usize
-    }
-}
-
-fn encode_size(dst: &mut [u8], n: usize, bounds: &Bounds) {
+pub(crate) const fn bytes_to_store_size_bounded(bounds: &Bounds) -> u32 {
     if bounds.is_fixed_size {
-        return;
-    }
-
-    if bounds.max_size <= u8::MAX as u32 {
-        dst[0] = n as u8;
-    } else if bounds.max_size <= u16::MAX as u32 {
-        dst[0..2].copy_from_slice(&(n as u16).to_be_bytes());
+        0
     } else {
-        dst[0..4].copy_from_slice(&(n as u32).to_be_bytes());
+        bytes_to_store_size(bounds.max_size as usize) as u32
     }
 }
 
-pub(crate) const fn bytes_to_store_size(bounds: &Bounds) -> u32 {
-    if bounds.is_fixed_size {
-        0
-    } else if bounds.max_size <= u8::MAX as u32 {
+const fn bytes_to_store_size(bytes_size: usize) -> usize {
+    if bytes_size <= u8::MAX as usize {
         1
-    } else if bounds.max_size <= u16::MAX as u32 {
+    } else if bytes_size <= u16::MAX as usize {
         2
     } else {
         4