From 19a2bfeaade9652a6c30ccf5f6302be4939ce757 Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Thu, 13 Dec 2018 09:33:45 -0500 Subject: [PATCH] More conversation viewmodel perf improvements. --- .../ConversationView/ConversationViewModel.m | 129 +++++++++++------ SignalMessaging/utils/ThreadUtil.m | 134 +++++++++--------- SignalServiceKit/src/Contacts/TSThread.m | 77 +++++----- .../src/Messages/OWSReadReceiptManager.m | 53 ++++--- 4 files changed, 215 insertions(+), 178 deletions(-) diff --git a/Signal/src/ViewControllers/ConversationView/ConversationViewModel.m b/Signal/src/ViewControllers/ConversationView/ConversationViewModel.m index c9720df78..2ff71f5ae 100644 --- a/Signal/src/ViewControllers/ConversationView/ConversationViewModel.m +++ b/Signal/src/ViewControllers/ConversationView/ConversationViewModel.m @@ -26,6 +26,22 @@ NS_ASSUME_NONNULL_BEGIN +@interface ConversationProfileState : NSObject + +@property (nonatomic) BOOL hasLocalProfile; +@property (nonatomic) BOOL isThreadInProfileWhitelist; +@property (nonatomic) BOOL hasUnwhitelistedMember; + +@end + +#pragma mark - + +@implementation ConversationProfileState + +@end + +#pragma mark - + @implementation ConversationUpdateItem - (instancetype)initWithUpdateItemType:(ConversationUpdateItemType)updateItemType @@ -146,6 +162,9 @@ static const int kYapDatabaseRangeMinLength = 0; @property (nonatomic, nullable) NSDate *collapseCutoffDate; @property (nonatomic, nullable) NSString *typingIndicatorsSender; +@property (nonatomic, nullable) ConversationProfileState *conversationProfileState; +@property (nonatomic) BOOL hasTooManyOutgoingMessagesToBlockCached; + @end #pragma mark - @@ -243,6 +262,10 @@ static const int kYapDatabaseRangeMinLength = 0; selector:@selector(blockListDidChange:) name:kNSNotificationName_BlockListDidChange object:nil]; + [[NSNotificationCenter defaultCenter] addObserver:self + selector:@selector(localProfileDidChange:) + name:kNSNotificationName_LocalProfileDidChange + object:nil]; } - (void)signalAccountsDidChange:(NSNotification *)notification @@ -256,17 +279,16 @@ static const int kYapDatabaseRangeMinLength = 0; { OWSAssertIsOnMainThread(); - // If profile whitelist just changed, we may want to hide a profile whitelist offer. - NSString *_Nullable recipientId = notification.userInfo[kNSNotificationKey_ProfileRecipientId]; - NSData *_Nullable groupId = notification.userInfo[kNSNotificationKey_ProfileGroupId]; - if (recipientId.length > 0 && [self.thread.recipientIdentifiers containsObject:recipientId]) { - [self updateForTransientItems]; - } else if (groupId.length > 0 && self.thread.isGroupThread) { - TSGroupThread *groupThread = (TSGroupThread *)self.thread; - if ([groupThread.groupModel.groupId isEqualToData:groupId]) { - [self updateForTransientItems]; - } - } + self.conversationProfileState = nil; + [self updateForTransientItems]; +} + +- (void)localProfileDidChange:(NSNotification *)notification +{ + OWSAssertIsOnMainThread(); + + self.conversationProfileState = nil; + [self updateForTransientItems]; } - (void)blockListDidChange:(id)notification @@ -929,8 +951,11 @@ static const int kYapDatabaseRangeMinLength = 0; #pragma mark - View Items -- (nullable OWSContactOffersInteraction *)tryToBuildContactOffersInteraction +- (void)ensureConversationProfileState { + if (self.conversationProfileState) { + return; + } // Many OWSProfileManager methods aren't safe to call from inside a database // transaction, so do this work now. @@ -946,23 +971,46 @@ static const int kYapDatabaseRangeMinLength = 0; } } - __block OWSContactOffersInteraction *_Nullable offers = nil; - [self.uiDatabaseConnection readWithBlock:^(YapDatabaseReadTransaction *transaction) { - offers = [self tryToBuildContactOffersInteractionWithTransaction:transaction - hasLocalProfile:hasLocalProfile - isThreadInProfileWhitelist:isThreadInProfileWhitelist - hasUnwhitelistedMember:hasUnwhitelistedMember]; - }]; - return offers; + ConversationProfileState *conversationProfileState = [ConversationProfileState new]; + conversationProfileState.hasLocalProfile = hasLocalProfile; + conversationProfileState.isThreadInProfileWhitelist = isThreadInProfileWhitelist; + conversationProfileState.hasUnwhitelistedMember = hasUnwhitelistedMember; + self.conversationProfileState = conversationProfileState; +} + +- (nullable TSInteraction *)firstCallOrMessageForLoadedInteractions:(NSArray *)loadedInteractions + +{ + for (TSInteraction *interaction in loadedInteractions) { + switch (interaction.interactionType) { + case OWSInteractionType_Unknown: + OWSFailDebug(@"Unknown interaction type."); + return nil; + case OWSInteractionType_IncomingMessage: + case OWSInteractionType_OutgoingMessage: + return interaction; + case OWSInteractionType_Error: + case OWSInteractionType_Info: + break; + case OWSInteractionType_Call: + case OWSInteractionType_Offer: + case OWSInteractionType_TypingIndicator: + break; + } + } + return nil; } - (nullable OWSContactOffersInteraction *) tryToBuildContactOffersInteractionWithTransaction:(YapDatabaseReadTransaction *)transaction - hasLocalProfile:(BOOL)hasLocalProfile - isThreadInProfileWhitelist:(BOOL)isThreadInProfileWhitelist - hasUnwhitelistedMember:(BOOL)hasUnwhitelistedMember + loadedInteractions:(NSArray *)loadedInteractions { OWSAssertDebug(transaction); + OWSAssertDebug(self.conversationProfileState); + + BOOL hasLocalProfile = self.conversationProfileState.hasLocalProfile; + BOOL isThreadInProfileWhitelist = self.conversationProfileState.isThreadInProfileWhitelist; + BOOL hasUnwhitelistedMember = self.conversationProfileState.hasUnwhitelistedMember; TSThread *thread = self.thread; BOOL isContactThread = [thread isKindOfClass:[TSContactThread class]]; @@ -977,27 +1025,22 @@ static const int kYapDatabaseRangeMinLength = 0; NSString *localNumber = [self.tsAccountManager localNumber]; OWSAssertDebug(localNumber.length > 0); - const int kMaxBlockOfferOutgoingMessageCount = 10; - - __block TSInteraction *firstCallOrMessage = nil; - [[transaction ext:TSMessageDatabaseViewExtensionName] - enumerateRowsInGroup:thread.uniqueId - usingBlock:^( - NSString *collection, NSString *key, id object, id metadata, NSUInteger index, BOOL *stop) { - OWSAssertDebug([object isKindOfClass:[TSInteraction class]]); - - if ([object isKindOfClass:[TSIncomingMessage class]] || - [object isKindOfClass:[TSOutgoingMessage class]] || [object isKindOfClass:[TSCall class]]) { - firstCallOrMessage = object; - *stop = YES; - } - }]; + TSInteraction *firstCallOrMessage = [self firstCallOrMessageForLoadedInteractions:loadedInteractions]; if (!firstCallOrMessage) { return nil; } - NSUInteger outgoingMessageCount = - [[TSDatabaseView threadOutgoingMessageDatabaseView:transaction] numberOfItemsInGroup:thread.uniqueId]; + BOOL hasTooManyOutgoingMessagesToBlock; + if (self.hasTooManyOutgoingMessagesToBlockCached) { + hasTooManyOutgoingMessagesToBlock = YES; + } else { + NSUInteger outgoingMessageCount = + [[TSDatabaseView threadOutgoingMessageDatabaseView:transaction] numberOfItemsInGroup:thread.uniqueId]; + + const int kMaxBlockOfferOutgoingMessageCount = 10; + hasTooManyOutgoingMessagesToBlock = (outgoingMessageCount > kMaxBlockOfferOutgoingMessageCount); + self.hasTooManyOutgoingMessagesToBlockCached = hasTooManyOutgoingMessagesToBlock; + } BOOL shouldHaveBlockOffer = YES; BOOL shouldHaveAddToContactsOffer = YES; @@ -1032,7 +1075,7 @@ static const int kYapDatabaseRangeMinLength = 0; } } - if (outgoingMessageCount > kMaxBlockOfferOutgoingMessageCount) { + if (hasTooManyOutgoingMessagesToBlock) { // If the user has sent more than N messages, don't show a block offer. shouldHaveBlockOffer = NO; } @@ -1101,7 +1144,7 @@ static const int kYapDatabaseRangeMinLength = 0; BOOL isGroupThread = self.thread.isGroupThread; ConversationStyle *conversationStyle = self.delegate.conversationStyle; - OWSContactOffersInteraction *_Nullable offers = [self tryToBuildContactOffersInteraction]; + [self ensureConversationProfileState]; __block BOOL hasError = NO; [self.uiDatabaseConnection readWithBlock:^(YapDatabaseReadTransaction *transaction) { @@ -1149,6 +1192,8 @@ static const int kYapDatabaseRangeMinLength = 0; [interactionIds addObject:interaction.uniqueId]; } + OWSContactOffersInteraction *_Nullable offers = + [self tryToBuildContactOffersInteractionWithTransaction:transaction loadedInteractions:interactions]; if (offers && [interactionIds containsObject:offers.beforeInteractionId]) { id offersItem = tryToAddViewItem(offers); if ([offersItem.interaction isKindOfClass:[OWSContactOffersInteraction class]]) { diff --git a/SignalMessaging/utils/ThreadUtil.m b/SignalMessaging/utils/ThreadUtil.m index 0274d1b56..fe65d0bb8 100644 --- a/SignalMessaging/utils/ThreadUtil.m +++ b/SignalMessaging/utils/ThreadUtil.m @@ -347,38 +347,38 @@ NS_ASSUME_NONNULL_BEGIN // We want to delete legacy and duplicate interactions. NSMutableArray *interactionsToDelete = [NSMutableArray new]; [[TSDatabaseView threadSpecialMessagesDatabaseView:transaction] - enumerateRowsInGroup:thread.uniqueId - usingBlock:^( - NSString *collection, NSString *key, id object, id metadata, NSUInteger index, BOOL *stop) { - - if ([object isKindOfClass:[OWSUnknownContactBlockOfferMessage class]]) { - // Delete this legacy interactions, which has been superseded by - // the OWSContactOffersInteraction. - [interactionsToDelete addObject:object]; - } else if ([object isKindOfClass:[OWSAddToContactsOfferMessage class]]) { - // Delete this legacy interactions, which has been superseded by - // the OWSContactOffersInteraction. - [interactionsToDelete addObject:object]; - } else if ([object isKindOfClass:[OWSAddToProfileWhitelistOfferMessage class]]) { - // Delete this legacy interactions, which has been superseded by - // the OWSContactOffersInteraction. - [interactionsToDelete addObject:object]; - } else if ([object isKindOfClass:[TSUnreadIndicatorInteraction class]]) { - // Remove obsolete unread indicator interactions. - [interactionsToDelete addObject:object]; - } else if ([object isKindOfClass:[OWSContactOffersInteraction class]]) { - // Remove obsolete contact offers. - [interactionsToDelete addObject:object]; - } else if ([object isKindOfClass:[TSInvalidIdentityKeyErrorMessage class]]) { - [blockingSafetyNumberChanges addObject:object]; - } else if ([object isKindOfClass:[TSErrorMessage class]]) { - TSErrorMessage *errorMessage = (TSErrorMessage *)object; - OWSAssertDebug(errorMessage.errorType == TSErrorMessageNonBlockingIdentityChange); - [nonBlockingSafetyNumberChanges addObject:errorMessage]; - } else { - OWSFailDebug(@"Unexpected dynamic interaction type: %@", [object class]); - } - }]; + enumerateKeysAndObjectsInGroup:thread.uniqueId + usingBlock:^( + NSString *collection, NSString *key, id object, NSUInteger index, BOOL *stop) { + if ([object isKindOfClass:[OWSUnknownContactBlockOfferMessage class]]) { + // Delete this legacy interactions, which has been superseded by + // the OWSContactOffersInteraction. + [interactionsToDelete addObject:object]; + } else if ([object isKindOfClass:[OWSAddToContactsOfferMessage class]]) { + // Delete this legacy interactions, which has been superseded by + // the OWSContactOffersInteraction. + [interactionsToDelete addObject:object]; + } else if ([object isKindOfClass:[OWSAddToProfileWhitelistOfferMessage class]]) { + // Delete this legacy interactions, which has been superseded by + // the OWSContactOffersInteraction. + [interactionsToDelete addObject:object]; + } else if ([object isKindOfClass:[TSUnreadIndicatorInteraction class]]) { + // Remove obsolete unread indicator interactions. + [interactionsToDelete addObject:object]; + } else if ([object isKindOfClass:[OWSContactOffersInteraction class]]) { + // Remove obsolete contact offers. + [interactionsToDelete addObject:object]; + } else if ([object isKindOfClass:[TSInvalidIdentityKeyErrorMessage class]]) { + [blockingSafetyNumberChanges addObject:object]; + } else if ([object isKindOfClass:[TSErrorMessage class]]) { + TSErrorMessage *errorMessage = (TSErrorMessage *)object; + OWSAssertDebug( + errorMessage.errorType == TSErrorMessageNonBlockingIdentityChange); + [nonBlockingSafetyNumberChanges addObject:errorMessage]; + } else { + OWSFailDebug(@"Unexpected dynamic interaction type: %@", [object class]); + } + }]; for (TSInteraction *interaction in interactionsToDelete) { OWSLogDebug(@"Cleaning up interaction: %@", [interaction class]); @@ -461,41 +461,41 @@ NS_ASSUME_NONNULL_BEGIN __block TSInteraction *interactionAfterUnreadIndicator = nil; __block BOOL hasMoreUnseenMessages = NO; [threadMessagesTransaction - enumerateRowsInGroup:thread.uniqueId - withOptions:NSEnumerationReverse - usingBlock:^( - NSString *collection, NSString *key, id object, id metadata, NSUInteger index, BOOL *stop) { - if (![object isKindOfClass:[TSInteraction class]]) { - OWSFailDebug(@"Expected a TSInteraction: %@", [object class]); - return; - } - - TSInteraction *interaction = (TSInteraction *)object; - - if (interaction.isDynamicInteraction) { - // Ignore dynamic interactions, if any. - return; - } - - if (interaction.timestampForSorting < firstUnseenInteractionTimestamp.unsignedLongLongValue) { - // By default we want the unread indicator to appear just before - // the first unread message. - *stop = YES; - return; - } - - visibleUnseenMessageCount++; - - interactionAfterUnreadIndicator = interaction; - - if (visibleUnseenMessageCount + 1 >= maxRangeSize) { - // If there are more unseen messages than can be displayed in the - // messages view, show the unread indicator at the top of the - // displayed messages. - *stop = YES; - hasMoreUnseenMessages = YES; - } - }]; + enumerateKeysAndObjectsInGroup:thread.uniqueId + withOptions:NSEnumerationReverse + usingBlock:^(NSString *collection, NSString *key, id object, NSUInteger index, BOOL *stop) { + if (![object isKindOfClass:[TSInteraction class]]) { + OWSFailDebug(@"Expected a TSInteraction: %@", [object class]); + return; + } + + TSInteraction *interaction = (TSInteraction *)object; + + if (interaction.isDynamicInteraction) { + // Ignore dynamic interactions, if any. + return; + } + + if (interaction.timestampForSorting + < firstUnseenInteractionTimestamp.unsignedLongLongValue) { + // By default we want the unread indicator to appear just before + // the first unread message. + *stop = YES; + return; + } + + visibleUnseenMessageCount++; + + interactionAfterUnreadIndicator = interaction; + + if (visibleUnseenMessageCount + 1 >= maxRangeSize) { + // If there are more unseen messages than can be displayed in the + // messages view, show the unread indicator at the top of the + // displayed messages. + *stop = YES; + hasMoreUnseenMessages = YES; + } + }]; if (!interactionAfterUnreadIndicator) { // If we can't find an interaction after the unread indicator, diff --git a/SignalServiceKit/src/Contacts/TSThread.m b/SignalServiceKit/src/Contacts/TSThread.m index 3a02af197..e07890428 100644 --- a/SignalServiceKit/src/Contacts/TSThread.m +++ b/SignalServiceKit/src/Contacts/TSThread.m @@ -212,14 +212,13 @@ ConversationColorName const kConversationColorName_Default = ConversationColorNa usingBlock:(void (^)(TSInteraction *interaction, YapDatabaseReadTransaction *transaction))block { - void (^interactionBlock)(NSString *, NSString *, id, id, NSUInteger, BOOL *) = ^void( - NSString *collection, NSString *key, id _Nonnull object, id _Nonnull metadata, NSUInteger index, BOOL *stop) { - TSInteraction *interaction = object; - block(interaction, transaction); - }; - YapDatabaseViewTransaction *interactionsByThread = [transaction ext:TSMessageDatabaseViewExtensionName]; - [interactionsByThread enumerateRowsInGroup:self.uniqueId usingBlock:interactionBlock]; + [interactionsByThread + enumerateKeysAndObjectsInGroup:self.uniqueId + usingBlock:^(NSString *collection, NSString *key, id object, NSUInteger index, BOOL *stop) { + TSInteraction *interaction = object; + block(interaction, transaction); + }]; } /** @@ -284,16 +283,14 @@ ConversationColorName const kConversationColorName_Default = ConversationColorNa { NSMutableArray> *messages = [NSMutableArray new]; [[TSDatabaseView unseenDatabaseViewExtension:transaction] - enumerateRowsInGroup:self.uniqueId - usingBlock:^( - NSString *collection, NSString *key, id object, id metadata, NSUInteger index, BOOL *stop) { - - if (![object conformsToProtocol:@protocol(OWSReadTracking)]) { - OWSFailDebug(@"Unexpected object in unseen messages: %@", [object class]); - return; - } - [messages addObject:(id)object]; - }]; + enumerateKeysAndObjectsInGroup:self.uniqueId + usingBlock:^(NSString *collection, NSString *key, id object, NSUInteger index, BOOL *stop) { + if (![object conformsToProtocol:@protocol(OWSReadTracking)]) { + OWSFailDebug(@"Unexpected object in unseen messages: %@", [object class]); + return; + } + [messages addObject:(id)object]; + }]; return [messages copy]; } @@ -320,29 +317,29 @@ ConversationColorName const kConversationColorName_Default = ConversationColorNa __block NSUInteger missedCount = 0; __block TSInteraction *last = nil; [[transaction ext:TSMessageDatabaseViewExtensionName] - enumerateRowsInGroup:self.uniqueId - withOptions:NSEnumerationReverse - usingBlock:^( - NSString *collection, NSString *key, id object, id metadata, NSUInteger index, BOOL *stop) { - - OWSAssertDebug([object isKindOfClass:[TSInteraction class]]); - - missedCount++; - TSInteraction *interaction = (TSInteraction *)object; - - if ([TSThread shouldInteractionAppearInInbox:interaction]) { - last = interaction; - - // For long ignored threads, with lots of SN changes this can get really slow. - // I see this in development because I have a lot of long forgotten threads with members - // who's test devices are constantly reinstalled. We could add a purpose-built DB view, - // but I think in the real world this is rare to be a hotspot. - if (missedCount > 50) { - OWSLogWarn(@"found last interaction for inbox after skipping %lu items", (unsigned long)missedCount); - } - *stop = YES; - } - }]; + enumerateKeysAndObjectsInGroup:self.uniqueId + withOptions:NSEnumerationReverse + usingBlock:^(NSString *collection, NSString *key, id object, NSUInteger index, BOOL *stop) { + OWSAssertDebug([object isKindOfClass:[TSInteraction class]]); + + missedCount++; + TSInteraction *interaction = (TSInteraction *)object; + + if ([TSThread shouldInteractionAppearInInbox:interaction]) { + last = interaction; + + // For long ignored threads, with lots of SN changes this can get really slow. + // I see this in development because I have a lot of long forgotten threads with + // members who's test devices are constantly reinstalled. We could add a + // purpose-built DB view, but I think in the real world this is rare to be a + // hotspot. + if (missedCount > 50) { + OWSLogWarn(@"found last interaction for inbox after skipping %lu items", + (unsigned long)missedCount); + } + *stop = YES; + } + }]; return last; } diff --git a/SignalServiceKit/src/Messages/OWSReadReceiptManager.m b/SignalServiceKit/src/Messages/OWSReadReceiptManager.m index af641b766..16d6e804c 100644 --- a/SignalServiceKit/src/Messages/OWSReadReceiptManager.m +++ b/SignalServiceKit/src/Messages/OWSReadReceiptManager.m @@ -478,35 +478,30 @@ NSString *const OWSReadReceiptManagerAreReadReceiptsEnabled = @"areReadReceiptsE NSMutableArray> *newlyReadList = [NSMutableArray new]; [[TSDatabaseView unseenDatabaseViewExtension:transaction] - enumerateRowsInGroup:thread.uniqueId - usingBlock:^(NSString *collection, - NSString *key, - id object, - id metadata, - NSUInteger index, - BOOL *stop) { - - if (![object conformsToProtocol:@protocol(OWSReadTracking)]) { - OWSFailDebug(@"Expected to conform to OWSReadTracking: object with class: %@ collection: %@ " - @"key: %@", - [object class], - collection, - key); - return; - } - id possiblyRead = (id)object; - - if (possiblyRead.timestampForSorting > timestamp) { - *stop = YES; - return; - } - - OWSAssertDebug(!possiblyRead.read); - OWSAssertDebug(possiblyRead.expireStartedAt == 0); - if (!possiblyRead.read) { - [newlyReadList addObject:possiblyRead]; - } - }]; + enumerateKeysAndObjectsInGroup:thread.uniqueId + usingBlock:^(NSString *collection, NSString *key, id object, NSUInteger index, BOOL *stop) { + if (![object conformsToProtocol:@protocol(OWSReadTracking)]) { + OWSFailDebug( + @"Expected to conform to OWSReadTracking: object with class: %@ collection: %@ " + @"key: %@", + [object class], + collection, + key); + return; + } + id possiblyRead = (id)object; + + if (possiblyRead.timestampForSorting > timestamp) { + *stop = YES; + return; + } + + OWSAssertDebug(!possiblyRead.read); + OWSAssertDebug(possiblyRead.expireStartedAt == 0); + if (!possiblyRead.read) { + [newlyReadList addObject:possiblyRead]; + } + }]; if (newlyReadList.count < 1) { return;