diff --git a/.github/workflows/benchmarks.yml b/.github/workflows/benchmarks.yml new file mode 100644 index 000000000..2699a0b32 --- /dev/null +++ b/.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' diff --git a/benchmarks/benches/crypto_bench.rs b/benchmarks/benches/crypto_bench.rs index 59017d64b..aa835ff20 100644 --- a/benchmarks/benches/crypto_bench.rs +++ b/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(); diff --git a/crates/matrix-sdk-crypto/src/store/integration_tests.rs b/crates/matrix-sdk-crypto/src/store/integration_tests.rs index 54d00632f..9007049dc 100644 --- a/crates/matrix-sdk-crypto/src/store/integration_tests.rs +++ b/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] diff --git a/crates/matrix-sdk-indexeddb/src/cryptostore.rs b/crates/matrix-sdk-indexeddb/src/cryptostore.rs index 1e1b6701f..1aa5988b3 100644 --- a/crates/matrix-sdk-indexeddb/src/cryptostore.rs +++ b/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, CryptoStoreError> { diff --git a/crates/matrix-sdk-sled/src/cryptostore.rs b/crates/matrix-sdk-sled/src/cryptostore.rs index c6e6d2c64..cf683ad29 100644 --- a/crates/matrix-sdk-sled/src/cryptostore.rs +++ b/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> {