From 96b79e9bf8b29d5a63c26b0ab580a0cca5e2446c Mon Sep 17 00:00:00 2001 From: George Kola Date: Fri, 10 Feb 2017 14:33:03 -0800 Subject: [PATCH] There is no need to use the main queue just for perform selector. We were using the main queue to queue up a perform selector and then the code [self sendStoredCrashReports] was immediately doing a dispatch_async. This unnecessary thread switching is not needed. We simplify the above logic and use dispatch_after to queue the block on the internal queue after a delay Note that main queue is typically more loaded and it is better for non-UI code to not use the main queue. This may also help improve crash log upload. This change also switches from @synchronized to dispatch_once as that is faster Reference: http://googlemac.blogspot.com/2006/10/synchronized-swimming.html BUG= Change-Id: I81035149cbbf13a3058ca3a11e6efd23980f19ad Reviewed-on: https://chromium-review.googlesource.com/441364 Reviewed-by: Joshua Peraza --- src/client/ios/BreakpadController.mm | 65 +++++++++++++--------------- 1 file changed, 31 insertions(+), 34 deletions(-) diff --git a/src/client/ios/BreakpadController.mm b/src/client/ios/BreakpadController.mm index dd71cff6..22f397a9 100644 --- a/src/client/ios/BreakpadController.mm +++ b/src/client/ios/BreakpadController.mm @@ -93,11 +93,12 @@ NSString* GetPlatform() { @implementation BreakpadController + (BreakpadController*)sharedInstance { - @synchronized(self) { - static BreakpadController* sharedInstance_ = - [[BreakpadController alloc] initSingleton]; - return sharedInstance_; - } + static dispatch_once_t onceToken; + static BreakpadController* sharedInstance ; + dispatch_once(&onceToken, ^{ + sharedInstance = [[BreakpadController alloc] initSingleton]; + }); + return sharedInstance; } - (id)init { @@ -179,10 +180,9 @@ NSString* GetPlatform() { enableUploads_ = YES; [self sendStoredCrashReports]; } else { + // disable the enableUpload_ flag. + // sendDelay checks this flag and disables the upload of logs by sendStoredCrashReports enableUploads_ = NO; - [NSObject cancelPreviousPerformRequestsWithTarget:self - selector:@selector(sendStoredCrashReports) - object:nil]; } }); } @@ -317,38 +317,35 @@ NSString* GetPlatform() { [userDefaults synchronize]; } +// This method must be called from the breakpad queue. - (void)sendStoredCrashReports { - dispatch_async(queue_, ^{ - if (BreakpadGetCrashReportCount(breakpadRef_) == 0) - return; + if (BreakpadGetCrashReportCount(breakpadRef_) == 0) + return; - int timeToWait = [self sendDelay]; + int timeToWait = [self sendDelay]; - // Unable to ever send report. - if (timeToWait == -1) - return; + // Unable to ever send report. + if (timeToWait == -1) + return; - // A report can be sent now. - if (timeToWait == 0) { - [self reportWillBeSent]; - BreakpadUploadNextReportWithParameters(breakpadRef_, - uploadTimeParameters_); + // A report can be sent now. + if (timeToWait == 0) { + [self reportWillBeSent]; + BreakpadUploadNextReportWithParameters(breakpadRef_, + uploadTimeParameters_); - // If more reports must be sent, make sure this method is called again. - if (BreakpadGetCrashReportCount(breakpadRef_) > 0) - timeToWait = uploadIntervalInSeconds_; - } + // If more reports must be sent, make sure this method is called again. + if (BreakpadGetCrashReportCount(breakpadRef_) > 0) + timeToWait = uploadIntervalInSeconds_; + } - // A report must be sent later. - if (timeToWait > 0) { - // performSelector: doesn't work on queue_ - dispatch_async(dispatch_get_main_queue(), ^{ - [self performSelector:@selector(sendStoredCrashReports) - withObject:nil - afterDelay:timeToWait]; - }); - } - }); + // A report must be sent later. + if (timeToWait > 0) { + dispatch_time_t delay = dispatch_time(DISPATCH_TIME_NOW, (int64_t)(timeToWait * NSEC_PER_SEC)); + dispatch_after(delay, queue_, ^{ + [self sendStoredCrashReports]; + }); + } } @end