From d9947bdd3308e5a257dba35378752df99c4e8726 Mon Sep 17 00:00:00 2001 From: George Nachman Date: Mon, 31 Jan 2022 14:47:26 -0800 Subject: [PATCH 1/2] Fix diffing profiles. When profile updates are applied, we diff them to avoid redundant saves and the resulting work (like reloading all cells of the conversation). The diff often failed spuriously because OWSUserProfileBadgeInfo did not implement `isEqual:` and instead relied on `NSObject`'s default implementation using pointer equality. This commit makes two changes: 1. Implement `-[OWSUserProfileBadgeInfo isEqual:]` 2. Force the ProfileBadge to be loaded prior to comparing snapshots so the comparison will be correct. This significantly reduces the number of database calls when opening a large chat. It may possibly increase the amount of DB work needed for smaller groups by loading the profile badge earlier. --- SignalServiceKit/src/Util/OWSUserProfile.m | 13 +++++++++++++ .../src/Util/OWSUserProfile.swift | 19 +++++++++++++++++++ 2 files changed, 32 insertions(+) diff --git a/SignalServiceKit/src/Util/OWSUserProfile.m b/SignalServiceKit/src/Util/OWSUserProfile.m index be19474485..d038086ade 100644 --- a/SignalServiceKit/src/Util/OWSUserProfile.m +++ b/SignalServiceKit/src/Util/OWSUserProfile.m @@ -646,6 +646,12 @@ NSString *NSStringForUserProfileWriter(UserProfileWriter userProfileWriter) [self anyUpdateWithTransaction:transaction block:^(OWSUserProfile *profile) { + // Load badges so they can be diffed. + [profile.profileBadgeInfo enumerateObjectsUsingBlock:^(OWSUserProfileBadgeInfo * _Nonnull badgeInfo, + NSUInteger idx, + BOOL * _Nonnull stop) { + (void)[badgeInfo fetchBadgeContentWithTransaction:transaction]; + }]; NSArray *avatarKeys = @[ @"avatarFileName", @"avatarUrlPath" ]; // self might be the latest instance, so take a "before" snapshot @@ -665,6 +671,13 @@ NSString *NSStringForUserProfileWriter(UserProfileWriter userProfileWriter) profile:profile userProfileWriter:userProfileWriter]; + // Load after updates in case anything changed. + [profile.profileBadgeInfo enumerateObjectsUsingBlock:^(OWSUserProfileBadgeInfo * _Nonnull badgeInfo, + NSUInteger idx, + BOOL * _Nonnull stop) { + (void)[badgeInfo fetchBadgeContentWithTransaction:transaction]; + }]; + profileKeyDidChange = ![NSObject isNullableObject:profileKeyBefore.keyData equalTo:profile.profileKey.keyData]; BOOL givenNameDidChange = ![NSObject isNullableObject:givenNameBefore diff --git a/SignalServiceKit/src/Util/OWSUserProfile.swift b/SignalServiceKit/src/Util/OWSUserProfile.swift index d8381d7948..dd4b75e9c9 100644 --- a/SignalServiceKit/src/Util/OWSUserProfile.swift +++ b/SignalServiceKit/src/Util/OWSUserProfile.swift @@ -54,6 +54,25 @@ public class OWSUserProfileBadgeInfo: NSObject, SDSSwiftSerializable { } return description } + + override public func isEqual(_ object: Any?) -> Bool { + guard let other = object as? OWSUserProfileBadgeInfo else { + return false + } + if badgeId != other.badgeId { + return false + } + if badge != other.badge { + return false + } + if expiration != other.expiration { + return false + } + if isVisible != other.isVisible { + return false + } + return true + } } @objc From 0d09397e360776e992042a8f270e69f7662e88fa Mon Sep 17 00:00:00 2001 From: George Nachman Date: Tue, 1 Feb 2022 14:07:43 -0800 Subject: [PATCH 2/2] Don't fetch & compare badges: badge ID is enough. --- SignalServiceKit/src/Util/OWSUserProfile.m | 13 ------------- SignalServiceKit/src/Util/OWSUserProfile.swift | 4 +--- 2 files changed, 1 insertion(+), 16 deletions(-) diff --git a/SignalServiceKit/src/Util/OWSUserProfile.m b/SignalServiceKit/src/Util/OWSUserProfile.m index d038086ade..be19474485 100644 --- a/SignalServiceKit/src/Util/OWSUserProfile.m +++ b/SignalServiceKit/src/Util/OWSUserProfile.m @@ -646,12 +646,6 @@ NSString *NSStringForUserProfileWriter(UserProfileWriter userProfileWriter) [self anyUpdateWithTransaction:transaction block:^(OWSUserProfile *profile) { - // Load badges so they can be diffed. - [profile.profileBadgeInfo enumerateObjectsUsingBlock:^(OWSUserProfileBadgeInfo * _Nonnull badgeInfo, - NSUInteger idx, - BOOL * _Nonnull stop) { - (void)[badgeInfo fetchBadgeContentWithTransaction:transaction]; - }]; NSArray *avatarKeys = @[ @"avatarFileName", @"avatarUrlPath" ]; // self might be the latest instance, so take a "before" snapshot @@ -671,13 +665,6 @@ NSString *NSStringForUserProfileWriter(UserProfileWriter userProfileWriter) profile:profile userProfileWriter:userProfileWriter]; - // Load after updates in case anything changed. - [profile.profileBadgeInfo enumerateObjectsUsingBlock:^(OWSUserProfileBadgeInfo * _Nonnull badgeInfo, - NSUInteger idx, - BOOL * _Nonnull stop) { - (void)[badgeInfo fetchBadgeContentWithTransaction:transaction]; - }]; - profileKeyDidChange = ![NSObject isNullableObject:profileKeyBefore.keyData equalTo:profile.profileKey.keyData]; BOOL givenNameDidChange = ![NSObject isNullableObject:givenNameBefore diff --git a/SignalServiceKit/src/Util/OWSUserProfile.swift b/SignalServiceKit/src/Util/OWSUserProfile.swift index dd4b75e9c9..15645d9a18 100644 --- a/SignalServiceKit/src/Util/OWSUserProfile.swift +++ b/SignalServiceKit/src/Util/OWSUserProfile.swift @@ -62,9 +62,7 @@ public class OWSUserProfileBadgeInfo: NSObject, SDSSwiftSerializable { if badgeId != other.badgeId { return false } - if badge != other.badge { - return false - } + // NOTE: We do not compare badges because the badgeId is good enough for equality purposes. if expiration != other.expiration { return false }