diff --git a/CHANGELOG.md b/CHANGELOG.md index 7dba5780c..fea7c2fe6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ ### Changes - Refactor: Remove the remaining AsRef #3669 +- Small speedup #3780 ### API-Changes - Add Python API to send reactions #3762 diff --git a/benches/receive_emails.rs b/benches/receive_emails.rs index 1e9e5aa43..ece40985e 100644 --- a/benches/receive_emails.rs +++ b/benches/receive_emails.rs @@ -38,11 +38,64 @@ Hello {i}", context } +/// Receive 100 emails that remove charlie@example.com and add +/// him back +async fn recv_groupmembership_emails(context: Context) -> Context { + for i in 0..50 { + let imf_raw = format!( + "Subject: Benchmark +Message-ID: Gr.OssSYnOFkhR.{i}@testrun.org +Date: Sat, 07 Dec 2019 19:00:27 +0000 +To: alice@example.com, b@example.com, c@example.com, d@example.com, e@example.com, f@example.com +From: sender@testrun.org +Chat-Version: 1.0 +Chat-Disposition-Notification-To: sender@testrun.org +Chat-User-Avatar: 0 +Chat-Group-Member-Added: charlie@example.com +In-Reply-To: Gr.OssSYnOFkhR.{i_dec}@testrun.org +MIME-Version: 1.0 + +Content-Type: text/plain; charset=utf-8; format=flowed; delsp=no + +Hello {i}", + i = i, + i_dec = i - 1, + ); + receive_imf(&context, black_box(imf_raw.as_bytes()), false) + .await + .unwrap(); + + let imf_raw = format!( + "Subject: Benchmark +Message-ID: Gr.OssSYnOFkhR.{i}@testrun.org +Date: Sat, 07 Dec 2019 19:00:27 +0000 +To: alice@example.com, b@example.com, c@example.com, d@example.com, e@example.com, f@example.com +From: sender@testrun.org +Chat-Version: 1.0 +Chat-Disposition-Notification-To: sender@testrun.org +Chat-User-Avatar: 0 +Chat-Group-Member-Removed: charlie@example.com +In-Reply-To: Gr.OssSYnOFkhR.{i_dec}@testrun.org +MIME-Version: 1.0 + +Content-Type: text/plain; charset=utf-8; format=flowed; delsp=no + +Hello {i}", + i = i, + i_dec = i - 1, + ); + receive_imf(&context, black_box(imf_raw.as_bytes()), false) + .await + .unwrap(); + } + context +} + async fn create_context() -> Context { let dir = tempdir().unwrap(); let dbfile = dir.path().join("db.sqlite"); let id = 100; - let context = Context::new(&dbfile, id, Events::new(), StockStrings::new()) + let context = Context::new(dbfile.as_path(), id, Events::new(), StockStrings::new()) .await .unwrap(); @@ -52,7 +105,7 @@ async fn create_context() -> Context { if backup.exists() { println!("Importing backup"); - imex(&context, ImexMode::ImportBackup, &backup, None) + imex(&context, ImexMode::ImportBackup, backup.as_path(), None) .await .unwrap(); } @@ -83,6 +136,20 @@ fn criterion_benchmark(c: &mut Criterion) { } }); }); + group.bench_function( + "Receive 100 Chat-Group-Member-{Added|Removed} messages", + |b| { + let rt = tokio::runtime::Runtime::new().unwrap(); + let context = rt.block_on(create_context()); + + b.to_async(&rt).iter(|| { + let ctx = context.clone(); + async move { + recv_groupmembership_emails(black_box(ctx)).await; + } + }); + }, + ); group.finish(); } diff --git a/src/chat.rs b/src/chat.rs index b348f7e09..9d1693924 100644 --- a/src/chat.rs +++ b/src/chat.rs @@ -2563,7 +2563,7 @@ pub async fn create_group_chat( let chat_id = ChatId::new(u32::try_from(row_id)?); if !is_contact_in_chat(context, chat_id, ContactId::SELF).await? { - add_to_chat_contacts_table(context, chat_id, ContactId::SELF).await?; + add_to_chat_contacts_table(context, chat_id, &[ContactId::SELF]).await?; } context.emit_msgs_changed_without_ids(); @@ -2624,19 +2624,25 @@ pub async fn create_broadcast_list(context: &Context) -> Result { Ok(chat_id) } -/// Adds a contact to the `chats_contacts` table. +/// Adds contacts to the `chats_contacts` table. pub(crate) async fn add_to_chat_contacts_table( context: &Context, chat_id: ChatId, - contact_id: ContactId, + contact_ids: &[ContactId], ) -> Result<()> { context .sql - .execute( - "INSERT INTO chats_contacts (chat_id, contact_id) VALUES(?, ?)", - paramsv![chat_id, contact_id], - ) + .transaction(move |transaction| { + for contact_id in contact_ids { + transaction.execute( + "INSERT OR IGNORE INTO chats_contacts (chat_id, contact_id) VALUES(?, ?)", + paramsv![chat_id, contact_id], + )?; + } + Ok(()) + }) .await?; + Ok(()) } @@ -2738,7 +2744,7 @@ pub(crate) async fn add_contact_to_chat_ex( if is_contact_in_chat(context, chat_id, contact_id).await? { return Ok(false); } - add_to_chat_contacts_table(context, chat_id, contact_id).await?; + add_to_chat_contacts_table(context, chat_id, &[contact_id]).await?; } if chat.typ == Chattype::Group && chat.is_promoted() { msg.viewtype = Viewtype::Text; diff --git a/src/peerstate.rs b/src/peerstate.rs index dd70f2b8a..e9591c789 100644 --- a/src/peerstate.rs +++ b/src/peerstate.rs @@ -4,7 +4,7 @@ use std::collections::HashSet; use std::fmt; use crate::aheader::{Aheader, EncryptPreference}; -use crate::chat::{self, is_contact_in_chat, Chat}; +use crate::chat::{self, Chat}; use crate::chatlist::Chatlist; use crate::constants::Chattype; use crate::contact::{addr_cmp, Contact, Origin}; @@ -523,9 +523,7 @@ impl Peerstate { let (new_contact_id, _) = Contact::add_or_lookup(context, "", new_addr, Origin::IncomingUnknownFrom) .await?; - if !is_contact_in_chat(context, *chat_id, new_contact_id).await? { - chat::add_to_chat_contacts_table(context, *chat_id, new_contact_id).await?; - } + chat::add_to_chat_contacts_table(context, *chat_id, &[new_contact_id]).await?; context.emit_event(EventType::ChatModified(*chat_id)); } diff --git a/src/receive_imf.rs b/src/receive_imf.rs index 6fbfdddf0..f677ee9d8 100644 --- a/src/receive_imf.rs +++ b/src/receive_imf.rs @@ -1530,19 +1530,13 @@ async fn create_or_lookup_group( chat_id_blocked = create_blocked; // Create initial member list. - chat::add_to_chat_contacts_table(context, new_chat_id, ContactId::SELF).await?; - if !from_id.is_special() && !chat::is_contact_in_chat(context, new_chat_id, from_id).await? - { - chat::add_to_chat_contacts_table(context, new_chat_id, from_id).await?; - } - for &to_id in to_ids.iter() { - info!(context, "adding to={:?} to chat id={}", to_id, new_chat_id); - if to_id != ContactId::SELF - && !chat::is_contact_in_chat(context, new_chat_id, to_id).await? - { - chat::add_to_chat_contacts_table(context, new_chat_id, to_id).await?; - } + let mut members = vec![ContactId::SELF]; + if !from_id.is_special() { + members.push(from_id); } + members.extend(to_ids); + members.dedup(); + chat::add_to_chat_contacts_table(context, new_chat_id, &members).await?; // once, we have protected-chats explained in UI, we can uncomment the following lines. // ("verified groups" did not add a message anyway) @@ -1698,6 +1692,7 @@ async fn apply_group_changes( .update_timestamp(context, Param::MemberListTimestamp, sent_timestamp) .await? { + let mut members_to_add = vec![]; if removed_id.is_some() || !chat::is_contact_in_chat(context, chat_id, ContactId::SELF).await? { @@ -1712,26 +1707,23 @@ async fn apply_group_changes( ) .await?; - if removed_id != Some(ContactId::SELF) { - chat::add_to_chat_contacts_table(context, chat_id, ContactId::SELF).await?; - } + members_to_add.push(ContactId::SELF); } - if !from_id.is_special() - && from_id != ContactId::SELF - && !chat::is_contact_in_chat(context, chat_id, from_id).await? - && removed_id != Some(from_id) - { - chat::add_to_chat_contacts_table(context, chat_id, from_id).await?; + + if !from_id.is_special() { + members_to_add.push(from_id); } - for &to_id in to_ids.iter() { - if to_id != ContactId::SELF - && !chat::is_contact_in_chat(context, chat_id, to_id).await? - && removed_id != Some(to_id) - { - info!(context, "adding to={:?} to chat id={}", to_id, chat_id); - chat::add_to_chat_contacts_table(context, chat_id, to_id).await?; - } + members_to_add.extend(to_ids); + if let Some(removed_id) = removed_id { + members_to_add.retain(|id| *id != removed_id); } + members_to_add.dedup(); + + info!( + context, + "adding {:?} to chat id={}", members_to_add, chat_id + ); + chat::add_to_chat_contacts_table(context, chat_id, &members_to_add).await?; send_event_chat_modified = true; } } @@ -1883,7 +1875,7 @@ async fn create_or_lookup_mailinglist( ) })?; - chat::add_to_chat_contacts_table(context, chat_id, ContactId::SELF).await?; + chat::add_to_chat_contacts_table(context, chat_id, &[ContactId::SELF]).await?; Ok(Some((chat_id, Blocked::Request))) } else { info!(context, "creating list forbidden by caller"); @@ -2013,9 +2005,7 @@ async fn create_adhoc_group( None, ) .await?; - for &member_id in member_ids.iter() { - chat::add_to_chat_contacts_table(context, new_chat_id, member_id).await?; - } + chat::add_to_chat_contacts_table(context, new_chat_id, member_ids).await?; context.emit_event(EventType::ChatModified(new_chat_id)); @@ -5160,13 +5150,10 @@ Reply from different address chat::add_to_chat_contacts_table( &bob, group_id, - bob.add_or_lookup_contact(&alice1).await.id, - ) - .await?; - chat::add_to_chat_contacts_table( - &bob, - group_id, - Contact::create(&bob, "", "charlie@example.org").await?, + &[ + bob.add_or_lookup_contact(&alice1).await.id, + Contact::create(&bob, "", "charlie@example.org").await?, + ], ) .await?; @@ -5247,7 +5234,7 @@ Reply from different address chat::add_to_chat_contacts_table( &bob, group_id, - bob.add_or_lookup_contact(&alice).await.id, + &[bob.add_or_lookup_contact(&alice).await.id], ) .await?; diff --git a/src/securejoin/bob.rs b/src/securejoin/bob.rs index a0f048b83..9162220ca 100644 --- a/src/securejoin/bob.rs +++ b/src/securejoin/bob.rs @@ -60,7 +60,7 @@ pub(super) async fn start_protocol(context: &Context, invite: QrInvite) -> Resul // TODO: how does this group become usable? let group_chat_id = state.joining_chat_id(context).await?; if !is_contact_in_chat(context, group_chat_id, invite.contact_id()).await? { - chat::add_to_chat_contacts_table(context, group_chat_id, invite.contact_id()) + chat::add_to_chat_contacts_table(context, group_chat_id, &[invite.contact_id()]) .await?; } let msg = stock_str::secure_join_started(context, invite.contact_id()).await; diff --git a/src/sql.rs b/src/sql.rs index d1e1c7e2d..4dacb5daf 100644 --- a/src/sql.rs +++ b/src/sql.rs @@ -432,7 +432,7 @@ impl Sql { pub async fn transaction(&self, callback: G) -> Result where H: Send + 'static, - G: Send + 'static + FnOnce(&mut rusqlite::Transaction<'_>) -> Result, + G: Send + FnOnce(&mut rusqlite::Transaction<'_>) -> Result, { let mut conn = self.get_conn().await?; tokio::task::block_in_place(move || { diff --git a/src/sql/migrations.rs b/src/sql/migrations.rs index fcb69cbac..0b5a46da8 100644 --- a/src/sql/migrations.rs +++ b/src/sql/migrations.rs @@ -649,6 +649,17 @@ CREATE INDEX smtp_messageid ON imap(rfc724_mid); ) .await?; } + if dbversion < 95 { + sql.execute_migration( + "CREATE TABLE new_chats_contacts (chat_id INTEGER, contact_id INTEGER, UNIQUE(chat_id, contact_id));\ + INSERT OR IGNORE INTO new_chats_contacts SELECT * FROM chats_contacts;\ + DROP TABLE chats_contacts;\ + ALTER TABLE new_chats_contacts RENAME TO chats_contacts;\ + CREATE INDEX chats_contacts_index1 ON chats_contacts (chat_id);\ + CREATE INDEX chats_contacts_index2 ON chats_contacts (contact_id);", + 95 + ).await?; + } let new_version = sql .get_raw_config_int(VERSION_CFG)