Skip to content

Commit

Permalink
impr: Cache installationID async (#3601)
Browse files Browse the repository at this point in the history
The SDK stores the installationID in a file. When the SDK launches, it
caches the installationID async to avoid file IO on the main thread.

Fixes GH-3575
  • Loading branch information
philipphofmann authored Feb 1, 2024
1 parent 6e11345 commit 1c96132
Show file tree
Hide file tree
Showing 6 changed files with 111 additions and 12 deletions.
7 changes: 7 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -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
Expand Down
4 changes: 4 additions & 0 deletions Sources/Sentry/SentryClient.m
Original file line number Diff line number Diff line change
Expand Up @@ -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];
}
Expand Down
42 changes: 35 additions & 7 deletions Sources/Sentry/SentryInstallation.m
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
#import "SentryInstallation.h"
#import "SentryDefines.h"
#import "SentryDependencyContainer.h"
#import "SentryDispatchQueueWrapper.h"
#import "SentryLog.h"

NS_ASSUME_NONNULL_BEGIN
Expand All @@ -8,6 +10,7 @@
SentryInstallation ()
@property (class, nonatomic, readonly)
NSMutableDictionary<NSString *, NSString *> *installationStringsByCacheDirectoryPaths;

@end

@implementation SentryInstallation
Expand All @@ -31,32 +34,57 @@ + (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;
return installationString;
}
}

+ (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
4 changes: 4 additions & 0 deletions Sources/Sentry/include/SentryInstallation.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
23 changes: 23 additions & 0 deletions Tests/SentryTests/SentryClientTests.swift
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import Nimble
import Sentry
import SentryTestUtils
import XCTest
Expand Down Expand Up @@ -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)
}
Expand Down
43 changes: 38 additions & 5 deletions Tests/SentryTests/State/SentryInstallationTests.swift
Original file line number Diff line number Diff line change
@@ -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() {
Expand All @@ -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

}
}

0 comments on commit 1c96132

Please sign in to comment.