From 41e6eaeafcad41d17d446f3e50736980acdafa24 Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Wed, 10 Jan 2018 12:51:29 -0500 Subject: [PATCH 1/6] Skip redundant sync messages. --- SignalServiceKit/src/Devices/OWSDevice.m | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/SignalServiceKit/src/Devices/OWSDevice.m b/SignalServiceKit/src/Devices/OWSDevice.m index fbbe81ff4..e3c16dda0 100644 --- a/SignalServiceKit/src/Devices/OWSDevice.m +++ b/SignalServiceKit/src/Devices/OWSDevice.m @@ -1,5 +1,5 @@ // -// Copyright (c) 2017 Open Whisper Systems. All rights reserved. +// Copyright (c) 2018 Open Whisper Systems. All rights reserved. // #import "OWSDevice.h" @@ -52,7 +52,7 @@ NSString *const kTSStorageManager_MayHaveLinkedDevices = @"kTSStorageManager_May if (!self.mayHaveLinkedDevicesCached) { self.mayHaveLinkedDevicesCached = @([dbConnection boolForKey:kTSStorageManager_MayHaveLinkedDevices inCollection:kTSStorageManager_OWSDeviceCollection - defaultValue:YES]); + defaultValue:NO]); } return [self.mayHaveLinkedDevicesCached boolValue]; From a2b67a17fd902362b5ed80433a0475417702c053 Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Wed, 10 Jan 2018 12:59:02 -0500 Subject: [PATCH 2/6] Skip redundant sync messages. --- SignalServiceKit/src/Devices/OWSDevice.h | 3 +- SignalServiceKit/src/Devices/OWSDevice.m | 30 +++++++++---------- .../src/Messages/OWSMessageManager.m | 1 + 3 files changed, 18 insertions(+), 16 deletions(-) diff --git a/SignalServiceKit/src/Devices/OWSDevice.h b/SignalServiceKit/src/Devices/OWSDevice.h index ba4fc39cc..11b11804c 100644 --- a/SignalServiceKit/src/Devices/OWSDevice.h +++ b/SignalServiceKit/src/Devices/OWSDevice.h @@ -1,5 +1,5 @@ // -// Copyright (c) 2017 Open Whisper Systems. All rights reserved. +// Copyright (c) 2018 Open Whisper Systems. All rights reserved. // #import "TSYapDatabaseObject.h" @@ -17,6 +17,7 @@ extern uint32_t const OWSDevicePrimaryDeviceId; - (BOOL)mayHaveLinkedDevices:(YapDatabaseConnection *)dbConnection; - (void)setMayHaveLinkedDevices:(BOOL)value dbConnection:(YapDatabaseConnection *)dbConnection; +- (void)setMayHaveLinkedDevices:(BOOL)value transaction:(YapDatabaseReadWriteTransaction *)transaction; - (BOOL)hasReceivedSyncMessageInLastSeconds:(NSTimeInterval)intervalSeconds; - (void)setHasReceivedSyncMessage; diff --git a/SignalServiceKit/src/Devices/OWSDevice.m b/SignalServiceKit/src/Devices/OWSDevice.m index e3c16dda0..1295fc03b 100644 --- a/SignalServiceKit/src/Devices/OWSDevice.m +++ b/SignalServiceKit/src/Devices/OWSDevice.m @@ -19,7 +19,6 @@ NSString *const kTSStorageManager_MayHaveLinkedDevices = @"kTSStorageManager_May @interface OWSDeviceManager () -@property (atomic, nullable) NSNumber *mayHaveLinkedDevicesCached; @property (atomic) NSDate *lastReceivedSyncMessage; @end @@ -49,13 +48,9 @@ NSString *const kTSStorageManager_MayHaveLinkedDevices = @"kTSStorageManager_May @synchronized(self) { - if (!self.mayHaveLinkedDevicesCached) { - self.mayHaveLinkedDevicesCached = @([dbConnection boolForKey:kTSStorageManager_MayHaveLinkedDevices - inCollection:kTSStorageManager_OWSDeviceCollection - defaultValue:NO]); - } - - return [self.mayHaveLinkedDevicesCached boolValue]; + return [dbConnection boolForKey:kTSStorageManager_MayHaveLinkedDevices + inCollection:kTSStorageManager_OWSDeviceCollection + defaultValue:NO]; } } @@ -63,15 +58,20 @@ NSString *const kTSStorageManager_MayHaveLinkedDevices = @"kTSStorageManager_May { OWSAssert(dbConnection); + [dbConnection asyncReadWriteWithBlock:^(YapDatabaseReadWriteTransaction *_Nonnull transaction) { + [self setMayHaveLinkedDevices:value transaction:transaction]; + }]; +} + +- (void)setMayHaveLinkedDevices:(BOOL)value transaction:(YapDatabaseReadWriteTransaction *)transaction +{ + OWSAssert(transaction); + @synchronized(self) { - self.mayHaveLinkedDevicesCached = @(value); - - [dbConnection asyncReadWriteWithBlock:^(YapDatabaseReadWriteTransaction *_Nonnull transaction) { - [transaction setObject:@(value) - forKey:kTSStorageManager_MayHaveLinkedDevices - inCollection:kTSStorageManager_OWSDeviceCollection]; - }]; + [transaction setObject:@(value) + forKey:kTSStorageManager_MayHaveLinkedDevices + inCollection:kTSStorageManager_OWSDeviceCollection]; } } diff --git a/SignalServiceKit/src/Messages/OWSMessageManager.m b/SignalServiceKit/src/Messages/OWSMessageManager.m index 2e6951ab8..335097c6f 100644 --- a/SignalServiceKit/src/Messages/OWSMessageManager.m +++ b/SignalServiceKit/src/Messages/OWSMessageManager.m @@ -278,6 +278,7 @@ NS_ASSUME_NONNULL_BEGIN [self handleIncomingEnvelope:envelope withSyncMessage:content.syncMessage transaction:transaction]; [[OWSDeviceManager sharedManager] setHasReceivedSyncMessage]; + [OWSDeviceManager.sharedManager setMayHaveLinkedDevices:YES transaction:transaction]; } else if (content.hasDataMessage) { [self handleIncomingEnvelope:envelope withDataMessage:content.dataMessage transaction:transaction]; } else if (content.hasCallMessage) { From c308e25115c61eca00ef8d0d789f5bd53ff1b8f3 Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Wed, 10 Jan 2018 13:00:11 -0500 Subject: [PATCH 3/6] Skip redundant sync messages. --- SignalServiceKit/src/Devices/OWSDevice.m | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/SignalServiceKit/src/Devices/OWSDevice.m b/SignalServiceKit/src/Devices/OWSDevice.m index 1295fc03b..4c1df0063 100644 --- a/SignalServiceKit/src/Devices/OWSDevice.m +++ b/SignalServiceKit/src/Devices/OWSDevice.m @@ -46,12 +46,9 @@ NSString *const kTSStorageManager_MayHaveLinkedDevices = @"kTSStorageManager_May { OWSAssert(dbConnection); - @synchronized(self) - { - return [dbConnection boolForKey:kTSStorageManager_MayHaveLinkedDevices - inCollection:kTSStorageManager_OWSDeviceCollection - defaultValue:NO]; - } + return [dbConnection boolForKey:kTSStorageManager_MayHaveLinkedDevices + inCollection:kTSStorageManager_OWSDeviceCollection + defaultValue:NO]; } - (void)setMayHaveLinkedDevices:(BOOL)value dbConnection:(YapDatabaseConnection *)dbConnection @@ -67,12 +64,9 @@ NSString *const kTSStorageManager_MayHaveLinkedDevices = @"kTSStorageManager_May { OWSAssert(transaction); - @synchronized(self) - { - [transaction setObject:@(value) - forKey:kTSStorageManager_MayHaveLinkedDevices - inCollection:kTSStorageManager_OWSDeviceCollection]; - } + [transaction setObject:@(value) + forKey:kTSStorageManager_MayHaveLinkedDevices + inCollection:kTSStorageManager_OWSDeviceCollection]; } - (BOOL)hasReceivedSyncMessageInLastSeconds:(NSTimeInterval)intervalSeconds From d81d85c386b139076908d362cb1e374e4f2d2c0f Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Thu, 11 Jan 2018 10:25:13 -0500 Subject: [PATCH 4/6] Respond to CR. --- .../OWSLinkDeviceViewController.m | 4 +- .../OWSLinkedDevicesTableViewController.m | 4 +- SignalServiceKit/src/Devices/OWSDevice.h | 3 +- SignalServiceKit/src/Devices/OWSDevice.m | 40 +++++++++++-------- .../src/Messages/OWSMessageManager.m | 1 - .../src/Messages/OWSMessageSender.m | 4 +- 6 files changed, 30 insertions(+), 26 deletions(-) diff --git a/Signal/src/ViewControllers/OWSLinkDeviceViewController.m b/Signal/src/ViewControllers/OWSLinkDeviceViewController.m index 471823d7a..2c178d215 100644 --- a/Signal/src/ViewControllers/OWSLinkDeviceViewController.m +++ b/Signal/src/ViewControllers/OWSLinkDeviceViewController.m @@ -1,5 +1,5 @@ // -// Copyright (c) 2017 Open Whisper Systems. All rights reserved. +// Copyright (c) 2018 Open Whisper Systems. All rights reserved. // #import "OWSLinkDeviceViewController.h" @@ -147,7 +147,7 @@ NS_ASSUME_NONNULL_BEGIN - (void)provisionWithParser:(OWSDeviceProvisioningURLParser *)parser { // Optimistically set this flag. - [OWSDeviceManager.sharedManager setMayHaveLinkedDevices:YES dbConnection:self.dbConnection]; + [OWSDeviceManager.sharedManager setMayHaveLinkedDevices]; ECKeyPair *_Nullable identityKeyPair = [[OWSIdentityManager sharedManager] identityKeyPair]; OWSAssert(identityKeyPair); diff --git a/Signal/src/ViewControllers/OWSLinkedDevicesTableViewController.m b/Signal/src/ViewControllers/OWSLinkedDevicesTableViewController.m index 2e315133d..d73af0574 100644 --- a/Signal/src/ViewControllers/OWSLinkedDevicesTableViewController.m +++ b/Signal/src/ViewControllers/OWSLinkedDevicesTableViewController.m @@ -146,9 +146,7 @@ int const OWSLinkedDevicesTableViewControllerSectionAddDevice = 1; if (devices.count > 1) { // Setting this flag here shouldn't be necessary, but we do so // because the "cost" is low and it will improve robustness. - [OWSDeviceManager.sharedManager - setMayHaveLinkedDevices:YES - dbConnection:[[TSStorageManager sharedManager] newDatabaseConnection]]; + [OWSDeviceManager.sharedManager setMayHaveLinkedDevices]; } if (devices.count > [OWSDevice numberOfKeysInCollection]) { diff --git a/SignalServiceKit/src/Devices/OWSDevice.h b/SignalServiceKit/src/Devices/OWSDevice.h index 11b11804c..e3b3f2259 100644 --- a/SignalServiceKit/src/Devices/OWSDevice.h +++ b/SignalServiceKit/src/Devices/OWSDevice.h @@ -16,8 +16,7 @@ extern uint32_t const OWSDevicePrimaryDeviceId; + (instancetype)sharedManager; - (BOOL)mayHaveLinkedDevices:(YapDatabaseConnection *)dbConnection; -- (void)setMayHaveLinkedDevices:(BOOL)value dbConnection:(YapDatabaseConnection *)dbConnection; -- (void)setMayHaveLinkedDevices:(BOOL)value transaction:(YapDatabaseReadWriteTransaction *)transaction; +- (void)setMayHaveLinkedDevices; - (BOOL)hasReceivedSyncMessageInLastSeconds:(NSTimeInterval)intervalSeconds; - (void)setHasReceivedSyncMessage; diff --git a/SignalServiceKit/src/Devices/OWSDevice.m b/SignalServiceKit/src/Devices/OWSDevice.m index 4c1df0063..056018dbc 100644 --- a/SignalServiceKit/src/Devices/OWSDevice.m +++ b/SignalServiceKit/src/Devices/OWSDevice.m @@ -19,6 +19,7 @@ NSString *const kTSStorageManager_MayHaveLinkedDevices = @"kTSStorageManager_May @interface OWSDeviceManager () +@property (atomic, nullable) NSNumber *mayHaveLinkedDevicesCached; @property (atomic) NSDate *lastReceivedSyncMessage; @end @@ -46,27 +47,32 @@ NSString *const kTSStorageManager_MayHaveLinkedDevices = @"kTSStorageManager_May { OWSAssert(dbConnection); - return [dbConnection boolForKey:kTSStorageManager_MayHaveLinkedDevices - inCollection:kTSStorageManager_OWSDeviceCollection - defaultValue:NO]; -} - -- (void)setMayHaveLinkedDevices:(BOOL)value dbConnection:(YapDatabaseConnection *)dbConnection -{ - OWSAssert(dbConnection); + // We don't bother synchronizing around + if (!self.mayHaveLinkedDevicesCached) { + self.mayHaveLinkedDevicesCached = @([dbConnection boolForKey:kTSStorageManager_MayHaveLinkedDevices + inCollection:kTSStorageManager_OWSDeviceCollection + defaultValue:YES]); + } - [dbConnection asyncReadWriteWithBlock:^(YapDatabaseReadWriteTransaction *_Nonnull transaction) { - [self setMayHaveLinkedDevices:value transaction:transaction]; - }]; + return [self.mayHaveLinkedDevicesCached boolValue]; } -- (void)setMayHaveLinkedDevices:(BOOL)value transaction:(YapDatabaseReadWriteTransaction *)transaction +- (void)setMayHaveLinkedDevices { - OWSAssert(transaction); + if (self.mayHaveLinkedDevicesCached != nil && self.mayHaveLinkedDevicesCached.boolValue) { + // Skip redundant writes. + return; + } - [transaction setObject:@(value) - forKey:kTSStorageManager_MayHaveLinkedDevices - inCollection:kTSStorageManager_OWSDeviceCollection]; + self.mayHaveLinkedDevicesCached = @(YES); + + // Note that we write async to avoid opening transactions within transactions. + [TSStorageManager.sharedManager.newDatabaseConnection + asyncReadWriteWithBlock:^(YapDatabaseReadWriteTransaction *_Nonnull transaction) { + [transaction setObject:@(YES) + forKey:kTSStorageManager_MayHaveLinkedDevices + inCollection:kTSStorageManager_OWSDeviceCollection]; + }]; } - (BOOL)hasReceivedSyncMessageInLastSeconds:(NSTimeInterval)intervalSeconds @@ -77,6 +83,8 @@ NSString *const kTSStorageManager_MayHaveLinkedDevices = @"kTSStorageManager_May - (void)setHasReceivedSyncMessage { self.lastReceivedSyncMessage = [NSDate new]; + + [self setMayHaveLinkedDevices]; } @end diff --git a/SignalServiceKit/src/Messages/OWSMessageManager.m b/SignalServiceKit/src/Messages/OWSMessageManager.m index 335097c6f..2e6951ab8 100644 --- a/SignalServiceKit/src/Messages/OWSMessageManager.m +++ b/SignalServiceKit/src/Messages/OWSMessageManager.m @@ -278,7 +278,6 @@ NS_ASSUME_NONNULL_BEGIN [self handleIncomingEnvelope:envelope withSyncMessage:content.syncMessage transaction:transaction]; [[OWSDeviceManager sharedManager] setHasReceivedSyncMessage]; - [OWSDeviceManager.sharedManager setMayHaveLinkedDevices:YES transaction:transaction]; } else if (content.hasDataMessage) { [self handleIncomingEnvelope:envelope withDataMessage:content.dataMessage transaction:transaction]; } else if (content.hasCallMessage) { diff --git a/SignalServiceKit/src/Messages/OWSMessageSender.m b/SignalServiceKit/src/Messages/OWSMessageSender.m index 82e8044d6..c9d80975b 100644 --- a/SignalServiceKit/src/Messages/OWSMessageSender.m +++ b/SignalServiceKit/src/Messages/OWSMessageSender.m @@ -1,5 +1,5 @@ // -// Copyright (c) 2017 Open Whisper Systems. All rights reserved. +// Copyright (c) 2018 Open Whisper Systems. All rights reserved. // #import "OWSMessageSender.h" @@ -1090,7 +1090,7 @@ NSString *const OWSMessageSenderRateLimitedException = @"RateLimitedException"; if (missingDevices.count > 0) { NSString *localNumber = [TSAccountManager localNumber]; if ([localNumber isEqualToString:recipient.uniqueId]) { - [OWSDeviceManager.sharedManager setMayHaveLinkedDevices:YES dbConnection:self.dbConnection]; + [OWSDeviceManager.sharedManager setMayHaveLinkedDevices]; } } From 3f2bee83832230d70bfbb98533f8d398cdce1845 Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Thu, 11 Jan 2018 10:27:02 -0500 Subject: [PATCH 5/6] Respond to CR. --- SignalServiceKit/src/Devices/OWSDevice.m | 32 +++++++++++++++--------- 1 file changed, 20 insertions(+), 12 deletions(-) diff --git a/SignalServiceKit/src/Devices/OWSDevice.m b/SignalServiceKit/src/Devices/OWSDevice.m index 056018dbc..04dce30fe 100644 --- a/SignalServiceKit/src/Devices/OWSDevice.m +++ b/SignalServiceKit/src/Devices/OWSDevice.m @@ -19,7 +19,9 @@ NSString *const kTSStorageManager_MayHaveLinkedDevices = @"kTSStorageManager_May @interface OWSDeviceManager () +// This property should only be accessed while synchronized on self. @property (atomic, nullable) NSNumber *mayHaveLinkedDevicesCached; + @property (atomic) NSDate *lastReceivedSyncMessage; @end @@ -47,24 +49,30 @@ NSString *const kTSStorageManager_MayHaveLinkedDevices = @"kTSStorageManager_May { OWSAssert(dbConnection); - // We don't bother synchronizing around - if (!self.mayHaveLinkedDevicesCached) { - self.mayHaveLinkedDevicesCached = @([dbConnection boolForKey:kTSStorageManager_MayHaveLinkedDevices - inCollection:kTSStorageManager_OWSDeviceCollection - defaultValue:YES]); - } + @synchronized(self) + { + // We don't bother synchronizing around + if (!self.mayHaveLinkedDevicesCached) { + self.mayHaveLinkedDevicesCached = @([dbConnection boolForKey:kTSStorageManager_MayHaveLinkedDevices + inCollection:kTSStorageManager_OWSDeviceCollection + defaultValue:YES]); + } - return [self.mayHaveLinkedDevicesCached boolValue]; + return [self.mayHaveLinkedDevicesCached boolValue]; + } } - (void)setMayHaveLinkedDevices { - if (self.mayHaveLinkedDevicesCached != nil && self.mayHaveLinkedDevicesCached.boolValue) { - // Skip redundant writes. - return; - } + @synchronized(self) + { + if (self.mayHaveLinkedDevicesCached != nil && self.mayHaveLinkedDevicesCached.boolValue) { + // Skip redundant writes. + return; + } - self.mayHaveLinkedDevicesCached = @(YES); + self.mayHaveLinkedDevicesCached = @(YES); + } // Note that we write async to avoid opening transactions within transactions. [TSStorageManager.sharedManager.newDatabaseConnection From fec2410ac2bc4b4a9aea8c718182c3cf3db0bd3f Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Thu, 11 Jan 2018 10:37:47 -0500 Subject: [PATCH 6/6] Respond to CR. --- SignalServiceKit/src/Devices/OWSDevice.m | 1 - 1 file changed, 1 deletion(-) diff --git a/SignalServiceKit/src/Devices/OWSDevice.m b/SignalServiceKit/src/Devices/OWSDevice.m index 04dce30fe..f47d0c133 100644 --- a/SignalServiceKit/src/Devices/OWSDevice.m +++ b/SignalServiceKit/src/Devices/OWSDevice.m @@ -51,7 +51,6 @@ NSString *const kTSStorageManager_MayHaveLinkedDevices = @"kTSStorageManager_May @synchronized(self) { - // We don't bother synchronizing around if (!self.mayHaveLinkedDevicesCached) { self.mayHaveLinkedDevicesCached = @([dbConnection boolForKey:kTSStorageManager_MayHaveLinkedDevices inCollection:kTSStorageManager_OWSDeviceCollection