Merge pull request #626 from matrix-org/gnunicorn/issue565

Fixing benchmarking, 

fixes #565

.github/workflows/benchmarks.yml

@@ -0,0 +1,56 @@
+name: Benchmarks
+
+on:
+  workflow_dispatch:
+
+jobs:
+  benchmarks:
+    name: Run Benchmarks
+    runs-on: ubuntu-latest
+    environment: matrix-rust-bot
+    if: github.event_name == 'push' || !github.event.pull_request.draft
+
+    steps:
+    - name: Checkout the repo
+      uses: actions/checkout@v2
+
+    - name: Install Rust
+      uses: actions-rs/toolchain@v1
+      with:
+        toolchain: nightly
+        components: rustfmt
+        profile: minimal
+        override: true
+
+    - name: Run Benchmarks
+      run: cargo bench | tee benchmark-output.txt
+
+    - name: Check benchmark result for PR
+      if: github.event_name == 'pull_request'
+      uses: benchmark-action/github-action-benchmark@v1
+      with:
+        name: Rust Benchmark
+        tool: 'cargo'
+        output-file-path: benchmark-output.txt
+        auto-push: false
+        # comment to alert the user this has gone bad
+        github-token: ${{ secrets.MRB_ACCESS_TOKEN }}
+        alert-threshold: '120%'
+        comment-on-alert: true
+        fail-threshold: '150%'
+        fail-on-alert: true
+
+    - name: Store benchmark result
+      if: github.event_name != 'pull_request'
+      uses: benchmark-action/github-action-benchmark@v1
+      with:
+        name: Rust Benchmark
+        tool: 'cargo'
+        output-file-path: benchmark-output.txt
+        github-token: ${{ secrets.GITHUB_TOKEN }}
+        auto-push: true
+        # Show alert with commit comment on detecting possible performance regression
+        alert-threshold: '150%'
+        comment-on-alert: true
+        fail-on-alert: true
+        alert-comment-cc-users: '@gnunicornBen,@jplatte,@poljar'

benchmarks/benches/crypto_bench.rs

@@ -71,7 +71,7 @@ pub fn keys_query(c: &mut Criterion) {
     });
 
     let dir = tempfile::tempdir().unwrap();
-    let store = Box::new(SledCryptoStore::open_with_passphrase(dir, None).unwrap());
+    let store = Box::new(SledCryptoStore::open_with_passphrase(dir.path(), None).unwrap());
     let machine =
         runtime.block_on(OlmMachine::with_store(alice_id(), alice_device_id(), store)).unwrap();
 
@@ -118,7 +118,8 @@ pub fn keys_claiming(c: &mut Criterion) {
         b.iter_batched(
             || {
                 let dir = tempfile::tempdir().unwrap();
-                let store = Box::new(SledCryptoStore::open_with_passphrase(dir, None).unwrap());
+                let store =
+                    Box::new(SledCryptoStore::open_with_passphrase(dir.path(), None).unwrap());
 
                 let machine = runtime
                     .block_on(OlmMachine::with_store(alice_id(), alice_device_id(), store))
@@ -180,7 +181,7 @@ pub fn room_key_sharing(c: &mut Criterion) {
         })
     });
     let dir = tempfile::tempdir().unwrap();
-    let store = Box::new(SledCryptoStore::open_with_passphrase(dir, None).unwrap());
+    let store = Box::new(SledCryptoStore::open_with_passphrase(dir.path(), None).unwrap());
 
     let machine =
         runtime.block_on(OlmMachine::with_store(alice_id(), alice_device_id(), store)).unwrap();
@@ -235,7 +236,7 @@ pub fn devices_missing_sessions_collecting(c: &mut Criterion) {
     });
 
     let dir = tempfile::tempdir().unwrap();
-    let store = Box::new(SledCryptoStore::open_with_passphrase(dir, None).unwrap());
+    let store = Box::new(SledCryptoStore::open_with_passphrase(dir.path(), None).unwrap());
 
     let machine =
         runtime.block_on(OlmMachine::with_store(alice_id(), alice_device_id(), store)).unwrap();

crates/matrix-sdk-crypto/src/store/integration_tests.rs

@@ -65,13 +65,26 @@ macro_rules! cryptostore_integration_tests {
                 (alice, session)
             }
 
+            #[async_test]
+            async fn save_account_via_generic_save() {
+                let store = get_store("save_account_via_generic".to_owned(), None).await;
+                assert!(store.get_account_info().is_none());
+                assert!(store.load_account().await.unwrap().is_none());
+                let account = get_account();
+
+                store.save_changes(Changes { account: Some(account), ..Default::default() } ).await.expect("Can't save account");
+                assert!(store.get_account_info().is_some());
+            }
+
             #[async_test]
             async fn save_account() {
                 let store = get_store("save_account".to_owned(), None).await;
+                assert!(store.get_account_info().is_none());
                 assert!(store.load_account().await.unwrap().is_none());
                 let account = get_account();
 
                 store.save_account(account).await.expect("Can't save account");
+                assert!(store.get_account_info().is_some());
             }
 
             #[async_test]

crates/matrix-sdk-indexeddb/src/cryptostore.rs

@@ -311,8 +311,18 @@ impl IndexeddbStore {
         let tx =
             self.inner.transaction_on_multi_with_mode(&stores, IdbTransactionMode::Readwrite)?;
 
-        let account_pickle =
-            if let Some(a) = changes.account { Some(a.pickle().await) } else { None };
+        let account_pickle = if let Some(account) = changes.account {
+            let account_info = AccountInfo {
+                user_id: account.user_id.clone(),
+                device_id: account.device_id.clone(),
+                identity_keys: account.identity_keys.clone(),
+            };
+
+            *self.account_info.write().unwrap() = Some(account_info);
+            Some(account.pickle().await)
+        } else {
+            None
+        };
 
         let private_identity_pickle =
             if let Some(i) = changes.private_identity { Some(i.pickle().await?) } else { None };
@@ -889,17 +899,9 @@ impl CryptoStore for IndexeddbStore {
     }
 
     async fn save_account(&self, account: ReadOnlyAccount) -> Result<(), CryptoStoreError> {
-        let account_info = AccountInfo {
-            user_id: account.user_id.clone(),
-            device_id: account.device_id.clone(),
-            identity_keys: account.identity_keys.clone(),
-        };
-
-        *self.account_info.write().unwrap() = Some(account_info);
-
-        let changes = Changes { account: Some(account), ..Default::default() };
-
-        self.save_changes(changes).await.map_err(|e| e.into())
+        self.save_changes(Changes { account: Some(account), ..Default::default() })
+            .await
+            .map_err(|e| e.into())
     }
 
     async fn load_identity(&self) -> Result<Option<PrivateCrossSigningIdentity>, CryptoStoreError> {

crates/matrix-sdk-sled/src/cryptostore.rs

@@ -440,8 +440,18 @@ impl SledStore {
     }
 
     async fn save_changes(&self, changes: Changes) -> Result<()> {
-        let account_pickle =
-            if let Some(a) = changes.account { Some(a.pickle().await) } else { None };
+        let account_pickle = if let Some(account) = changes.account {
+            let account_info = AccountInfo {
+                user_id: account.user_id.clone(),
+                device_id: account.device_id.clone(),
+                identity_keys: account.identity_keys.clone(),
+            };
+
+            *self.account_info.write().unwrap() = Some(account_info);
+            Some(account.pickle().await)
+        } else {
+            None
+        };
 
         let private_identity_pickle =
             if let Some(i) = changes.private_identity { Some(i.pickle().await?) } else { None };
@@ -705,17 +715,7 @@ impl CryptoStore for SledStore {
     }
 
     async fn save_account(&self, account: ReadOnlyAccount) -> Result<()> {
-        let account_info = AccountInfo {
-            user_id: account.user_id.clone(),
-            device_id: account.device_id.clone(),
-            identity_keys: account.identity_keys.clone(),
-        };
-
-        *self.account_info.write().unwrap() = Some(account_info);
-
-        let changes = Changes { account: Some(account), ..Default::default() };
-
-        self.save_changes(changes).await
+        self.save_changes(Changes { account: Some(account), ..Default::default() }).await
     }
 
     async fn load_identity(&self) -> Result<Option<PrivateCrossSigningIdentity>> {