From 47480b7eee5b9e60652ef003e57cccd8171a2db0 Mon Sep 17 00:00:00 2001 From: Evan Hahn Date: Wed, 29 Mar 2023 17:22:40 -0500 Subject: [PATCH] Speed up "mark read locally" optimization check This should cause no change in behavior. We had logic like this pseudocode: ``` unreadCount = db.countUnreadMessages() messagesWithUnreadReactionsCount = db.countMessagesWithUnreadReactions() if unreadCount == 0 && messagesWithUnreadReactionsCount == 0 { // Avoid unnecessary writes. return } db.markMessagesRead() ``` This optimization works but has two problems: 1. We don't actually need the count. We just need to know whether anything exists. 2. If there are unread messages, we should do the cleanup regardless, and we don't need to do the second check query. Now the pseudocode looks like this: ``` if db.hasUnreadMessages() || db.hasMessagesToMarkRead() { // Avoid unnecessary writes. return } db.markMessagesRead() ``` --- .../src/Messages/OWSReceiptManager.swift | 15 ++-- .../Database/Records/InteractionFinder.swift | 84 +++++++++---------- 2 files changed, 46 insertions(+), 53 deletions(-) diff --git a/SignalServiceKit/src/Messages/OWSReceiptManager.swift b/SignalServiceKit/src/Messages/OWSReceiptManager.swift index 3a58b6cc34..ff98743d83 100644 --- a/SignalServiceKit/src/Messages/OWSReceiptManager.swift +++ b/SignalServiceKit/src/Messages/OWSReceiptManager.swift @@ -280,16 +280,13 @@ public extension OWSReceiptManager { DispatchQueue.global().async { let interactionFinder = InteractionFinder(threadUniqueId: thread.uniqueId) - let (unreadCount, messagesWithUnreadReactionsCount) = self.databaseStorage.read { transaction in - ( - interactionFinder.countUnreadMessages(beforeSortId: sortId, - transaction: transaction.unwrapGrdbRead), - interactionFinder.countMessagesWithUnreadReactions(beforeSortId: sortId, - transaction: transaction.unwrapGrdbRead) + let hasMessagesToMarkRead = self.databaseStorage.read { transaction in + return interactionFinder.hasMessagesToMarkRead( + beforeSortId: sortId, + transaction: transaction.unwrapGrdbRead ) } - - if unreadCount == 0 && messagesWithUnreadReactionsCount == 0 { + guard hasMessagesToMarkRead else { // Avoid unnecessary writes. DispatchQueue.main.async(execute: completion) return @@ -308,7 +305,7 @@ public extension OWSReceiptManager { circumstance = .onThisDevice logSuffix = "" } - Logger.info("Marking \(unreadCount) received messages and \(messagesWithUnreadReactionsCount) sent messages with reactions as read locally\(logSuffix) (in batches of \(maxBatchSize))") + Logger.info("Marking received messages and sent messages with reactions as read locally\(logSuffix) (in batches of \(maxBatchSize))") var batchQuotaRemaining: Int repeat { diff --git a/SignalServiceKit/src/Storage/Database/Records/InteractionFinder.swift b/SignalServiceKit/src/Storage/Database/Records/InteractionFinder.swift index 5eaee16e6d..673c628b0b 100644 --- a/SignalServiceKit/src/Storage/Database/Records/InteractionFinder.swift +++ b/SignalServiceKit/src/Storage/Database/Records/InteractionFinder.swift @@ -459,32 +459,53 @@ public class InteractionFinder: NSObject, InteractionFinderAdapter { } } - @objc - public func countUnreadMessages(beforeSortId: UInt64, transaction: GRDBReadTransaction) -> UInt { - do { - let sql = """ - SELECT COUNT(*) + /// Do we have any messages to mark read in this thread before a given sort ID? + /// + /// See also: ``fetchUnreadMessages`` and ``fetchMessagesWithUnreadReactions``. + public func hasMessagesToMarkRead( + beforeSortId: UInt64, + transaction: GRDBReadTransaction + ) -> Bool { + let hasUnreadMessages = (try? Bool.fetchOne( + transaction.database, + sql: """ + SELECT EXISTS ( + SELECT 1 FROM \(InteractionRecord.databaseTableName) WHERE \(interactionColumn: .threadUniqueId) = ? AND \(interactionColumn: .id) <= ? AND \(sqlClauseForAllUnreadInteractions()) - """ + LIMIT 1 + ) + """, + arguments: [threadUniqueId, beforeSortId] + )) ?? false - guard let count = try UInt.fetchOne(transaction.database, - sql: sql, - arguments: [threadUniqueId, beforeSortId]) else { - owsFailDebug("count was unexpectedly nil") - return 0 - } - return count - } catch { - owsFailDebug("error: \(error)") - return 0 - } + lazy var hasOutgoingMessagesWithUnreadReactions = (try? Bool.fetchOne( + transaction.database, + sql: """ + SELECT EXISTS ( + SELECT 1 + FROM \(InteractionRecord.databaseTableName) AS interaction + INNER JOIN \(OWSReaction.databaseTableName) AS reaction + ON interaction.\(interactionColumn: .uniqueId) = reaction.\(OWSReaction.columnName(.uniqueMessageId)) + AND reaction.\(OWSReaction.columnName(.read)) IS 0 + WHERE interaction.\(interactionColumn: .recordType) IS \(SDSRecordType.outgoingMessage.rawValue) + AND interaction.\(interactionColumn: .threadUniqueId) = ? + AND interaction.\(interactionColumn: .id) <= ? + LIMIT 1 + ) + """, + arguments: [threadUniqueId, beforeSortId] + )) ?? false + + return hasUnreadMessages || hasOutgoingMessagesWithUnreadReactions } /// Enumerates all the unread interactions in this thread before a given sort id, /// sorted by sort id. + /// + /// See also: ``hasMessagesToMarkRead``. public func fetchUnreadMessages( beforeSortId: UInt64, transaction: GRDBReadTransaction @@ -512,35 +533,10 @@ public class InteractionFinder: NSObject, InteractionFinderAdapter { } } - @objc - public func countMessagesWithUnreadReactions(beforeSortId: UInt64, transaction: GRDBReadTransaction) -> UInt { - do { - let sql = """ - SELECT COUNT(DISTINCT interaction.\(interactionColumn: .id)) - FROM \(InteractionRecord.databaseTableName) AS interaction - INNER JOIN \(OWSReaction.databaseTableName) AS reaction - ON interaction.\(interactionColumn: .uniqueId) = reaction.\(OWSReaction.columnName(.uniqueMessageId)) - AND reaction.\(OWSReaction.columnName(.read)) IS 0 - WHERE interaction.\(interactionColumn: .recordType) IS \(SDSRecordType.outgoingMessage.rawValue) - AND interaction.\(interactionColumn: .threadUniqueId) = ? - AND interaction.\(interactionColumn: .id) <= ? - """ - - guard let count = try UInt.fetchOne(transaction.database, - sql: sql, - arguments: [threadUniqueId, beforeSortId]) else { - owsFailDebug("count was unexpectedly nil") - return 0 - } - return count - } catch { - owsFailDebug("error: \(error)") - return 0 - } - } - /// Returns all the messages with unread reactions in this thread before a given sort id, /// sorted by sort id. + /// + /// See also: ``hasMessagesToMarkRead``. public func fetchMessagesWithUnreadReactions( beforeSortId: UInt64, transaction: GRDBReadTransaction