From cf65cc3be591d51abefc3c88d73deb944f0d93b1 Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Tue, 20 Jun 2017 17:39:33 -0400 Subject: [PATCH 1/3] Improve perf of attachment stream file path upgrade. // FREEBIE --- src/Messages/Attachments/TSAttachmentStream.h | 2 ++ src/Messages/Attachments/TSAttachmentStream.m | 27 ++++++------------- 2 files changed, 10 insertions(+), 19 deletions(-) diff --git a/src/Messages/Attachments/TSAttachmentStream.h b/src/Messages/Attachments/TSAttachmentStream.h index c206198fd..2c763d37b 100644 --- a/src/Messages/Attachments/TSAttachmentStream.h +++ b/src/Messages/Attachments/TSAttachmentStream.h @@ -27,6 +27,8 @@ NS_ASSUME_NONNULL_BEGIN // This only applies for attachments being uploaded. @property (atomic) BOOL isUploaded; +@property (atomic) BOOL hasUnsavedFilePath; + #if TARGET_OS_IPHONE - (nullable UIImage *)image; #endif diff --git a/src/Messages/Attachments/TSAttachmentStream.m b/src/Messages/Attachments/TSAttachmentStream.m index 86d7ba36a..9365d0e98 100644 --- a/src/Messages/Attachments/TSAttachmentStream.m +++ b/src/Messages/Attachments/TSAttachmentStream.m @@ -129,29 +129,18 @@ NS_ASSUME_NONNULL_BEGIN OWSAssert(self.filePath); if (shouldPersist) { - // It's not ideal to do this asynchronously, but we can create a new transaction - // within initWithCoder: which will be called from within a transaction. - // - // We use a serial queue to ensure we don't spawn a ton of threads each doing - // database writes. - dispatch_async([TSAttachmentStream serialQueue], ^{ - [self.dbConnection readWriteWithBlock:^(YapDatabaseReadWriteTransaction *transaction) { - OWSAssert(transaction); - - [self saveWithTransaction:transaction]; - }]; - }); + self.hasUnsavedFilePath = YES; } } -+ (dispatch_queue_t)serialQueue ++ (MTLPropertyStorage)storageBehaviorForPropertyWithKey:(NSString *)propertyKey { - static dispatch_queue_t queue = nil; - static dispatch_once_t onceToken; - dispatch_once(&onceToken, ^{ - queue = dispatch_queue_create("org.whispersystems.attachment.stream", DISPATCH_QUEUE_SERIAL); - }); - return queue; + // Don't persist transient properties + if ([propertyKey isEqualToString:@"hasUnsavedFilePath"]) { + return MTLPropertyStorageNone; + } else { + return [super storageBehaviorForPropertyWithKey:propertyKey]; + } } #pragma mark - File Management From beb4ed71e740aa8b5902faf5702992b2a94c2531 Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Wed, 21 Jun 2017 09:58:19 -0400 Subject: [PATCH 2/3] Respond to CR. // FREEBIE --- src/Messages/Attachments/TSAttachmentStream.h | 4 +-- src/Messages/Attachments/TSAttachmentStream.m | 26 +++---------------- 2 files changed, 6 insertions(+), 24 deletions(-) diff --git a/src/Messages/Attachments/TSAttachmentStream.h b/src/Messages/Attachments/TSAttachmentStream.h index 2c763d37b..95b7b6f79 100644 --- a/src/Messages/Attachments/TSAttachmentStream.h +++ b/src/Messages/Attachments/TSAttachmentStream.h @@ -27,8 +27,6 @@ NS_ASSUME_NONNULL_BEGIN // This only applies for attachments being uploaded. @property (atomic) BOOL isUploaded; -@property (atomic) BOOL hasUnsavedFilePath; - #if TARGET_OS_IPHONE - (nullable UIImage *)image; #endif @@ -54,6 +52,8 @@ NS_ASSUME_NONNULL_BEGIN - (CGFloat)audioDurationSecondsWithTransaction:(YapDatabaseReadWriteTransaction *)transaction; - (CGFloat)audioDurationSecondsWithoutTransaction; +//- (BOOL)hasUnsavedFilePath; + @end NS_ASSUME_NONNULL_END diff --git a/src/Messages/Attachments/TSAttachmentStream.m b/src/Messages/Attachments/TSAttachmentStream.m index 9365d0e98..b490e30bb 100644 --- a/src/Messages/Attachments/TSAttachmentStream.m +++ b/src/Messages/Attachments/TSAttachmentStream.m @@ -42,8 +42,7 @@ NS_ASSUME_NONNULL_BEGIN // attachments which haven't been uploaded yet. _isUploaded = NO; - // This instance hasn't been persisted yet. - [self ensureFilePathAndPersist:NO]; + [self ensureFilePath]; return self; } @@ -64,8 +63,7 @@ NS_ASSUME_NONNULL_BEGIN _isUploaded = YES; self.attachmentType = pointer.attachmentType; - // This instance hasn't been persisted yet. - [self ensureFilePathAndPersist:NO]; + [self ensureFilePath]; return self; } @@ -77,9 +75,7 @@ NS_ASSUME_NONNULL_BEGIN return self; } - // This instance has been persisted, we need to - // update it in the database. - [self ensureFilePathAndPersist:YES]; + [self ensureFilePath]; return self; } @@ -97,7 +93,7 @@ NS_ASSUME_NONNULL_BEGIN } } -- (void)ensureFilePathAndPersist:(BOOL)shouldPersist +- (void)ensureFilePath { if (self.localRelativeFilePath) { return; @@ -127,20 +123,6 @@ NS_ASSUME_NONNULL_BEGIN self.localRelativeFilePath = localRelativeFilePath; OWSAssert(self.filePath); - - if (shouldPersist) { - self.hasUnsavedFilePath = YES; - } -} - -+ (MTLPropertyStorage)storageBehaviorForPropertyWithKey:(NSString *)propertyKey -{ - // Don't persist transient properties - if ([propertyKey isEqualToString:@"hasUnsavedFilePath"]) { - return MTLPropertyStorageNone; - } else { - return [super storageBehaviorForPropertyWithKey:propertyKey]; - } } #pragma mark - File Management From e86e175ceb096d1897dd4fdc079363c3665e9ca7 Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Wed, 21 Jun 2017 10:35:25 -0400 Subject: [PATCH 3/3] Respond to CR. // FREEBIE --- src/Messages/Attachments/TSAttachmentStream.h | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/Messages/Attachments/TSAttachmentStream.h b/src/Messages/Attachments/TSAttachmentStream.h index 95b7b6f79..c206198fd 100644 --- a/src/Messages/Attachments/TSAttachmentStream.h +++ b/src/Messages/Attachments/TSAttachmentStream.h @@ -52,8 +52,6 @@ NS_ASSUME_NONNULL_BEGIN - (CGFloat)audioDurationSecondsWithTransaction:(YapDatabaseReadWriteTransaction *)transaction; - (CGFloat)audioDurationSecondsWithoutTransaction; -//- (BOOL)hasUnsavedFilePath; - @end NS_ASSUME_NONNULL_END