From 7ca8f4c763e1683ce9ea84e66fb2c8e2d4252f55 Mon Sep 17 00:00:00 2001 From: trevor-signal <131492920+trevor-signal@users.noreply.github.com> Date: Thu, 7 Sep 2023 16:07:07 -0400 Subject: [PATCH] Improve bulk message deletion speed --- ts/services/expiringMessagesDeletion.ts | 22 ++++++++++------------ ts/sql/Client.ts | 16 ++++++++++++++-- ts/util/cleanup.ts | 10 ++++++++-- 3 files changed, 32 insertions(+), 16 deletions(-) diff --git a/ts/services/expiringMessagesDeletion.ts b/ts/services/expiringMessagesDeletion.ts index ceee06d72..2dd06f8b1 100644 --- a/ts/services/expiringMessagesDeletion.ts +++ b/ts/services/expiringMessagesDeletion.ts @@ -1,6 +1,7 @@ // Copyright 2016 Signal Messenger, LLC // SPDX-License-Identifier: AGPL-3.0-only +import { batch } from 'react-redux'; import { debounce } from 'lodash'; import type { MessageModel } from '../models/messages'; @@ -31,7 +32,6 @@ class ExpiringMessagesDeletionService { const messageIds: Array = []; const inMemoryMessages: Array = []; - const messageCleanup: Array> = []; messages.forEach(dbMessage => { const message = window.MessageController.register( @@ -40,22 +40,20 @@ class ExpiringMessagesDeletionService { ); messageIds.push(message.id); inMemoryMessages.push(message); - messageCleanup.push(message.cleanup()); }); - // We delete after the trigger to allow the conversation time to process - // the expiration before the message is removed from the database. await window.Signal.Data.removeMessages(messageIds); - await Promise.all(messageCleanup); - inMemoryMessages.forEach(message => { - window.SignalContext.log.info('Message expired', { - sentAt: message.get('sent_at'), + batch(() => { + inMemoryMessages.forEach(message => { + window.SignalContext.log.info('Message expired', { + sentAt: message.get('sent_at'), + }); + + // We do this to update the UI, if this message is being displayed somewhere + message.trigger('expired'); + window.reduxActions.conversations.messageExpired(message.id); }); - - // We do this to update the UI, if this message is being displayed somewhere - message.trigger('expired'); - window.reduxActions.conversations.messageExpired(message.id); }); if (messages.length > 0) { diff --git a/ts/sql/Client.ts b/ts/sql/Client.ts index 32ef61750..02e058e20 100644 --- a/ts/sql/Client.ts +++ b/ts/sql/Client.ts @@ -3,6 +3,7 @@ import { ipcRenderer as ipc } from 'electron'; import PQueue from 'p-queue'; +import { batch } from 'react-redux'; import { has, get, groupBy, isTypedArray, last, map, omit } from 'lodash'; @@ -23,7 +24,11 @@ import * as Errors from '../types/errors'; import type { StoredJob } from '../jobs/types'; import { formatJobForInsert } from '../jobs/formatJobForInsert'; -import { cleanupMessage } from '../util/cleanup'; +import { + cleanupMessage, + cleanupMessageFromMemory, + deleteMessageData, +} from '../util/cleanup'; import { drop } from '../util/drop'; import { ipcInvoke, doShutdown } from './channels'; @@ -590,11 +595,18 @@ async function removeMessage(id: string): Promise { async function _cleanupMessages( messages: ReadonlyArray ): Promise { + // First, remove messages from memory, so we can batch the updates in redux + batch(() => { + messages.forEach(message => cleanupMessageFromMemory(message)); + }); + + // Then, handle any asynchronous actions (e.g. deleting data from disk) const queue = new PQueue({ concurrency: 3, timeout: MINUTE * 30 }); drop( queue.addAll( messages.map( - (message: MessageAttributesType) => async () => cleanupMessage(message) + (message: MessageAttributesType) => async () => + deleteMessageData(message) ) ) ); diff --git a/ts/util/cleanup.ts b/ts/util/cleanup.ts index d571e1eed..b45f7b257 100644 --- a/ts/util/cleanup.ts +++ b/ts/util/cleanup.ts @@ -10,6 +10,14 @@ import * as log from '../logging/log'; export async function cleanupMessage( message: MessageAttributesType ): Promise { + cleanupMessageFromMemory(message); + await deleteMessageData(message); +} + +/** Removes a message from redux caches & backbone, but does NOT delete files on disk, + * story replies, edit histories, attachments, etc. Should ONLY be called in conjunction + * with deleteMessageData. */ +export function cleanupMessageFromMemory(message: MessageAttributesType): void { const { id, conversationId } = message; window.reduxActions?.conversations.messageDeleted(id, conversationId); @@ -18,8 +26,6 @@ export async function cleanupMessage( parentConversation?.debouncedUpdateLastMessage(); window.MessageController.unregister(id); - - await deleteMessageData(message); } async function cleanupStoryReplies(