diff --git a/CHANGELOG.md b/CHANGELOG.md index c0160ae7816..1799f13ff7a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,12 @@ # Changelog +## Unreleased + +### Improvements + +- Cache installationID async to avoid file IO on the main thread when starting the SDK (#3601) + + ## 8.20.0 ### Features diff --git a/Sources/Sentry/SentryClient.m b/Sources/Sentry/SentryClient.m index 73019c8da68..82e79df0475 100644 --- a/Sources/Sentry/SentryClient.m +++ b/Sources/Sentry/SentryClient.m @@ -151,6 +151,10 @@ - (instancetype)initWithOptions:(SentryOptions *)options self.timezone = timezone; self.attachmentProcessors = [[NSMutableArray alloc] init]; + // The SDK stores the installationID in a file. The first call requires file IO. To avoid + // executing this on the main thread, we cache the installationID async here. + [SentryInstallation cacheIDAsyncWithCacheDirectoryPath:options.cacheDirectoryPath]; + if (deleteOldEnvelopeItems) { [fileManager deleteOldEnvelopeItems]; } diff --git a/Sources/Sentry/SentryInstallation.m b/Sources/Sentry/SentryInstallation.m index 47170cb7431..7a0c64121ef 100644 --- a/Sources/Sentry/SentryInstallation.m +++ b/Sources/Sentry/SentryInstallation.m @@ -1,5 +1,7 @@ #import "SentryInstallation.h" #import "SentryDefines.h" +#import "SentryDependencyContainer.h" +#import "SentryDispatchQueueWrapper.h" #import "SentryLog.h" NS_ASSUME_NONNULL_BEGIN @@ -8,6 +10,7 @@ SentryInstallation () @property (class, nonatomic, readonly) NSMutableDictionary *installationStringsByCacheDirectoryPaths; + @end @implementation SentryInstallation @@ -31,25 +34,25 @@ + (NSString *)idWithCacheDirectoryPath:(NSString *)cacheDirectoryPath return installationString; } - NSString *cachePath = cacheDirectoryPath; - NSString *installationFilePath = [cachePath stringByAppendingPathComponent:@"INSTALLATION"]; - NSData *installationData = [NSData dataWithContentsOfFile:installationFilePath]; + installationString = + [SentryInstallation idWithCacheDirectoryPathNonCached:cacheDirectoryPath]; - if (nil == installationData) { + if (installationString == nil) { installationString = [NSUUID UUID].UUIDString; + NSData *installationStringData = [installationString dataUsingEncoding:NSUTF8StringEncoding]; NSFileManager *fileManager = [NSFileManager defaultManager]; + NSString *installationFilePath = + [SentryInstallation installationFilePath:cacheDirectoryPath]; + if (![fileManager createFileAtPath:installationFilePath contents:installationStringData attributes:nil]) { SENTRY_LOG_ERROR( @"Failed to store installationID file at path %@", installationFilePath); } - } else { - installationString = [[NSString alloc] initWithData:installationData - encoding:NSUTF8StringEncoding]; } self.installationStringsByCacheDirectoryPaths[cacheDirectoryPath] = installationString; @@ -57,6 +60,31 @@ + (NSString *)idWithCacheDirectoryPath:(NSString *)cacheDirectoryPath } } ++ (nullable NSString *)idWithCacheDirectoryPathNonCached:(NSString *)cacheDirectoryPath +{ + NSString *installationFilePath = [SentryInstallation installationFilePath:cacheDirectoryPath]; + + NSData *installationData = [NSData dataWithContentsOfFile:installationFilePath]; + + if (installationData != nil) { + return [[NSString alloc] initWithData:installationData encoding:NSUTF8StringEncoding]; + } else { + return nil; + } +} + ++ (void)cacheIDAsyncWithCacheDirectoryPath:(NSString *)cacheDirectoryPath +{ + [SentryDependencyContainer.sharedInstance.dispatchQueueWrapper dispatchAsyncWithBlock:^{ + [SentryInstallation idWithCacheDirectoryPath:cacheDirectoryPath]; + }]; +} + ++ (NSString *)installationFilePath:(NSString *)cacheDirectoryPath +{ + return [cacheDirectoryPath stringByAppendingPathComponent:@"INSTALLATION"]; +} + @end NS_ASSUME_NONNULL_END diff --git a/Sources/Sentry/include/SentryInstallation.h b/Sources/Sentry/include/SentryInstallation.h index 20ba50d5102..5f13d17af83 100644 --- a/Sources/Sentry/include/SentryInstallation.h +++ b/Sources/Sentry/include/SentryInstallation.h @@ -8,6 +8,10 @@ NS_ASSUME_NONNULL_BEGIN + (NSString *)idWithCacheDirectoryPath:(NSString *)cacheDirectoryPath; ++ (nullable NSString *)idWithCacheDirectoryPathNonCached:(NSString *)cacheDirectoryPath; + ++ (void)cacheIDAsyncWithCacheDirectoryPath:(NSString *)cacheDirectoryPath; + @end NS_ASSUME_NONNULL_END diff --git a/Tests/SentryTests/SentryClientTests.swift b/Tests/SentryTests/SentryClientTests.swift index a1648cbaa05..b650280d35f 100644 --- a/Tests/SentryTests/SentryClientTests.swift +++ b/Tests/SentryTests/SentryClientTests.swift @@ -1,3 +1,4 @@ +import Nimble import Sentry import SentryTestUtils import XCTest @@ -156,6 +157,28 @@ class SentryClientTest: XCTestCase { XCTAssertEqual(1, fileManager.deleteOldEnvelopeItemsInvocations.count) } + func testInitCachesInstallationIDAsync() throws { + let dispatchQueue = fixture.dispatchQueue + SentryDependencyContainer.sharedInstance().dispatchQueueWrapper = fixture.dispatchQueue + + let options = Options() + options.dsn = SentryClientTest.dsn + // We have to put our cache into a subfolder of the default path, because on macOS we can't delete the default cache folder + options.cacheDirectoryPath = "\(options.cacheDirectoryPath)/cache" + _ = SentryClient(options: options) + + expect(dispatchQueue.dispatchAsyncInvocations.count) == 1 + + let nonCachedID = SentryInstallation.id(withCacheDirectoryPathNonCached: options.cacheDirectoryPath) + + // We remove the file containing the installation ID, but the cached ID is still in memory + try FileManager().removeItem(atPath: options.cacheDirectoryPath) + + let cachedID = SentryInstallation.id(withCacheDirectoryPath: options.cacheDirectoryPath) + + expect(cachedID) == nonCachedID + } + func testClientIsEnabled() { XCTAssertTrue(fixture.getSut().isEnabled) } diff --git a/Tests/SentryTests/State/SentryInstallationTests.swift b/Tests/SentryTests/State/SentryInstallationTests.swift index dfe177ecae7..b48234375ec 100644 --- a/Tests/SentryTests/State/SentryInstallationTests.swift +++ b/Tests/SentryTests/State/SentryInstallationTests.swift @@ -1,19 +1,23 @@ +import Nimble +import SentryTestUtils import XCTest final class SentryInstallationTests: XCTestCase { - var basePath: String! + + // FileManager().temporaryDirectory already has a trailing slash + let basePath = "\(FileManager().temporaryDirectory)\(UUID().uuidString)" override func setUpWithError() throws { try super.setUpWithError() - // FileManager().temporaryDirectory already has a trailing slash - basePath = "\(FileManager().temporaryDirectory)\(UUID().uuidString)" try FileManager().createDirectory(atPath: basePath, withIntermediateDirectories: true) - print("base path: \(basePath!)") } override func tearDownWithError() throws { super.tearDown() - try FileManager().removeItem(atPath: basePath) + let fileManager = FileManager.default + if fileManager.fileExists(atPath: basePath) { + try FileManager().removeItem(atPath: basePath) + } } func testSentryInstallationId() { @@ -39,4 +43,33 @@ final class SentryInstallationTests: XCTestCase { SentryInstallation.installationStringsByCacheDirectoryPaths.removeAllObjects() XCTAssertEqual(id1, SentryInstallation.id(withCacheDirectoryPath: basePath)) } + + func testCacheIDAsync_ExecutesOnBackgroundThread() { + let dispatchQueue = TestSentryDispatchQueueWrapper() + SentryDependencyContainer.sharedInstance().dispatchQueueWrapper = dispatchQueue + + SentryInstallation.cacheIDAsync(withCacheDirectoryPath: basePath) + + expect(dispatchQueue.dispatchAsyncInvocations.count) == 1 + } + + func testCacheIDAsync_CashesID() throws { + let dispatchQueue = TestSentryDispatchQueueWrapper() + SentryDependencyContainer.sharedInstance().dispatchQueueWrapper = dispatchQueue + + SentryInstallation.cacheIDAsync(withCacheDirectoryPath: basePath) + + let nonCachedID = SentryInstallation.id(withCacheDirectoryPathNonCached: basePath) + + // We remove the file containing the installation ID, but the cached ID is still in memory + try FileManager().removeItem(atPath: basePath) + + let nonCachedIDAfterDeletingFile = SentryInstallation.id(withCacheDirectoryPathNonCached: basePath) + expect(nonCachedIDAfterDeletingFile) == nil + + let cachedID = SentryInstallation.id(withCacheDirectoryPath: basePath) + + expect(cachedID) == nonCachedID + + } }