From 2803694cdfdb6a180f833749b856318c9671e6d6 Mon Sep 17 00:00:00 2001 From: Benjamin Kampmann Date: Fri, 29 Apr 2022 17:54:53 +0200 Subject: [PATCH 01/10] ci(benchmarks): Adding Benchmarks to CI --- .github/workflows/benchmarks.yml | 63 ++++++++++++++++++++++++++++++++ 1 file changed, 63 insertions(+) create mode 100644 .github/workflows/benchmarks.yml diff --git a/.github/workflows/benchmarks.yml b/.github/workflows/benchmarks.yml new file mode 100644 index 000000000..a546b6379 --- /dev/null +++ b/.github/workflows/benchmarks.yml @@ -0,0 +1,63 @@ +name: Benchmarks + +on: + workflow_dispatch: + push: + branches: [main] + pull_request: + branches: [main] + types: + - opened + - reopened + - synchronize + - ready_for_review + +jobs: + benchmarks: + name: Check style + runs-on: ubuntu-latest + 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 + 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' From fb8ddfa07d48ecc9f4feda08dc15099945d93d86 Mon Sep 17 00:00:00 2001 From: Benjamin Kampmann Date: Fri, 29 Apr 2022 17:56:50 +0200 Subject: [PATCH 02/10] style: Fixing name --- .github/workflows/benchmarks.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/benchmarks.yml b/.github/workflows/benchmarks.yml index a546b6379..93b2b3d4a 100644 --- a/.github/workflows/benchmarks.yml +++ b/.github/workflows/benchmarks.yml @@ -14,7 +14,7 @@ on: jobs: benchmarks: - name: Check style + name: Run Benchmarks runs-on: ubuntu-latest if: github.event_name == 'push' || !github.event.pull_request.draft From 827f8c3e17736042731a30014a6d475949565eca Mon Sep 17 00:00:00 2001 From: Benjamin Kampmann Date: Mon, 2 May 2022 14:17:13 +0200 Subject: [PATCH 03/10] ci(benchmarking): Add github token to benchmarking --- .github/workflows/benchmarks.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/benchmarks.yml b/.github/workflows/benchmarks.yml index 93b2b3d4a..927fc3510 100644 --- a/.github/workflows/benchmarks.yml +++ b/.github/workflows/benchmarks.yml @@ -42,6 +42,7 @@ jobs: output-file-path: benchmark-output.txt auto-push: false # comment to alert the user this has gone bad + github-token: ${{ secrets.GITHUB_TOKEN }} alert-threshold: '120%' comment-on-alert: true fail-threshold: '150%' From a4ec6cb6d61269149da9097b120cb3e24fe81c5e Mon Sep 17 00:00:00 2001 From: Benjamin Kampmann Date: Mon, 2 May 2022 16:29:02 +0200 Subject: [PATCH 04/10] fix(sled-crypto): Ensure cryptostore.save_changes for account updates internal account_info --- .../src/store/integration_tests.rs | 13 ++++++++++ crates/matrix-sdk-sled/src/cryptostore.rs | 26 +++++++++---------- 2 files changed, 26 insertions(+), 13 deletions(-) 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-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> { From b6d71cbeba3c38a4ef7344c4dd8a501bb3ea6568 Mon Sep 17 00:00:00 2001 From: Benjamin Kampmann Date: Mon, 2 May 2022 16:29:50 +0200 Subject: [PATCH 05/10] fix(benchmarks): keep the temprorary dir until end of tests --- benchmarks/benches/crypto_bench.rs | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) 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(); From 54c93e4982def19c6a142b7c2fddce6f3fb36eea Mon Sep 17 00:00:00 2001 From: Benjamin Kampmann Date: Mon, 2 May 2022 16:30:30 +0200 Subject: [PATCH 06/10] ci(benchmarks): more specific cargo-bench params --- .github/workflows/benchmarks.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/benchmarks.yml b/.github/workflows/benchmarks.yml index 927fc3510..f79472fcd 100644 --- a/.github/workflows/benchmarks.yml +++ b/.github/workflows/benchmarks.yml @@ -31,7 +31,7 @@ jobs: override: true - name: Run Benchmarks - run: cargo bench | tee benchmark-output.txt + run: cargo bench --workspace --benches | tee benchmark-output.txt - name: Check benchmark result for PR if: github.event_name == 'pull_request' From 9763c13a34c01d98d8fae47cc6efceda67f1de12 Mon Sep 17 00:00:00 2001 From: Benjamin Kampmann Date: Mon, 2 May 2022 16:36:44 +0200 Subject: [PATCH 07/10] ci(benchmarks): Switch to Mr. B access token --- .github/workflows/benchmarks.yml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.github/workflows/benchmarks.yml b/.github/workflows/benchmarks.yml index f79472fcd..83290e700 100644 --- a/.github/workflows/benchmarks.yml +++ b/.github/workflows/benchmarks.yml @@ -16,6 +16,7 @@ jobs: benchmarks: name: Run Benchmarks runs-on: ubuntu-latest + environment: matrix-rust-bot if: github.event_name == 'push' || !github.event.pull_request.draft steps: @@ -42,7 +43,7 @@ jobs: output-file-path: benchmark-output.txt auto-push: false # comment to alert the user this has gone bad - github-token: ${{ secrets.GITHUB_TOKEN }} + github-token: ${{ secrets.MRB_ACCESS_TOKEN }} alert-threshold: '120%' comment-on-alert: true fail-threshold: '150%' From f20390036bdfb9734fcda2a1b25b997568383f43 Mon Sep 17 00:00:00 2001 From: Benjamin Kampmann Date: Mon, 2 May 2022 16:58:21 +0200 Subject: [PATCH 08/10] fix(indexeddb): Ensure internal account info is up to date on regular save_changes --- .../matrix-sdk-indexeddb/src/cryptostore.rs | 28 ++++++++++--------- 1 file changed, 15 insertions(+), 13 deletions(-) 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> { From 12894aff9942fe6cf094f2ad295fd57246148de7 Mon Sep 17 00:00:00 2001 From: Benjamin Kampmann Date: Mon, 2 May 2022 17:01:56 +0200 Subject: [PATCH 09/10] ci(benchmark): Fixing output format --- .github/workflows/benchmarks.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/benchmarks.yml b/.github/workflows/benchmarks.yml index 83290e700..7d39ad1d5 100644 --- a/.github/workflows/benchmarks.yml +++ b/.github/workflows/benchmarks.yml @@ -32,7 +32,7 @@ jobs: override: true - name: Run Benchmarks - run: cargo bench --workspace --benches | tee benchmark-output.txt + run: cargo bench | tee benchmark-output.txt - name: Check benchmark result for PR if: github.event_name == 'pull_request' From aad0cbe49653ee68c33d448fbbe566f0e85db536 Mon Sep 17 00:00:00 2001 From: Benjamin Kampmann Date: Tue, 3 May 2022 09:39:35 +0200 Subject: [PATCH 10/10] ci(benchmarking): disable benchmark runs other than manually requested --- .github/workflows/benchmarks.yml | 9 --------- 1 file changed, 9 deletions(-) diff --git a/.github/workflows/benchmarks.yml b/.github/workflows/benchmarks.yml index 7d39ad1d5..2699a0b32 100644 --- a/.github/workflows/benchmarks.yml +++ b/.github/workflows/benchmarks.yml @@ -2,15 +2,6 @@ name: Benchmarks on: workflow_dispatch: - push: - branches: [main] - pull_request: - branches: [main] - types: - - opened - - reopened - - synchronize - - ready_for_review jobs: benchmarks: