Skip to content

Commit

Permalink
Merge 43d0810 into 945053f
Browse files Browse the repository at this point in the history
  • Loading branch information
armcknight authored Feb 14, 2024
2 parents 945053f + 43d0810 commit b4dffe4
Show file tree
Hide file tree
Showing 7 changed files with 11 additions and 78 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ @implementation SentrySDKPerformanceBenchmarkTests

- (void)testCPUBenchmark
{
XCTSkipIf(isSimulator() && !isDebugging());
// XCTSkipIf(isSimulator() && !isDebugging());

NSMutableArray *results = [NSMutableArray array];
for (NSUInteger j = 0; j < 20; j++) {
Expand Down
10 changes: 2 additions & 8 deletions Sources/Sentry/Profiling/SentryLaunchProfiling.m
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,9 @@
# import "SentryTransactionContext.h"

BOOL isTracingAppLaunch;
SentryId *_Nullable appLaunchTraceId;
NSObject *appLaunchTraceLock;
uint64_t appLaunchSystemTime;
NSString *const kSentryLaunchProfileConfigKeyTracesSampleRate = @"traces";
NSString *const kSentryLaunchProfileConfigKeyProfilesSampleRate = @"profiles";
SentryTracer *_Nullable launchTracer;
static SentryTracer *_Nullable launchTracer;

# pragma mark - Private

Expand Down Expand Up @@ -129,11 +126,8 @@
[SentryLog configure:YES diagnosticLevel:kSentryLevelDebug];
# endif // defined(DEBUG)

appLaunchSystemTime = SentryDependencyContainer.sharedInstance.dateProvider.systemTime;
appLaunchTraceLock = [[NSObject alloc] init];
appLaunchTraceId = [[SentryId alloc] init];
SENTRY_LOG_INFO(@"Starting app launch profile.");

SENTRY_LOG_INFO(@"Starting app launch profile at %llu", appLaunchSystemTime);
SentryTransactionContext *context =
[[SentryTransactionContext alloc] initWithName:@"launch"
operation:@"app.lifecycle"
Expand Down
15 changes: 8 additions & 7 deletions Sources/Sentry/SentryProfiler.mm
Original file line number Diff line number Diff line change
Expand Up @@ -463,18 +463,19 @@ + (nullable SentryEnvelopeItem *)createEnvelopeItemForProfilePayload:
if (isTracingAppLaunch) {
SENTRY_LOG_DEBUG(@"Writing app launch profile.");
pathToWrite = [appSupportDirPath stringByAppendingPathComponent:@"launchProfile"];
if (!SENTRY_CASSERT_RETURN(![fm fileExistsAtPath:pathToWrite],
@"Already a launch profile file present; make sure to remove them right after "
@"using them, and that tests clean state in between so there isn't leftover config "
@"producing one when it isn't expected.")) {
return;
}
} else {
SENTRY_LOG_DEBUG(@"Overwriting last non-launch profile.");
pathToWrite = [appSupportDirPath stringByAppendingPathComponent:@"profile"];
}

SENTRY_LOG_DEBUG(@"Writing app launch profile to file.");
if ([fm fileExistsAtPath:pathToWrite]) {
SENTRY_LOG_DEBUG(@"Already a%@ profile file present; make sure to remove them right after "
@"using them, and that tests clean state in between so there isn't "
@"leftover config producing one when it isn't expected.",
isTracingAppLaunch ? @" launch" : @"");
}

SENTRY_LOG_DEBUG(@"Writing%@ profile to file.", isTracingAppLaunch ? @" launch" : @"");
SENTRY_CASSERT(
[data writeToFile:pathToWrite atomically:YES], @"Failed to write profile to test file");
}
Expand Down
9 changes: 0 additions & 9 deletions Sources/Sentry/SentryTracer.m
Original file line number Diff line number Diff line change
Expand Up @@ -609,15 +609,6 @@ - (void)finishInternal
#if SENTRY_TARGET_PROFILING_SUPPORTED
if (self.isProfiling) {
[self captureTransactionWithProfile:transaction];

// as long as this isn't used for any conditional branching logic, and is just being set to
// NO, we don't need to synchronize the read here
if (isTracingAppLaunch) {
@synchronized(appLaunchTraceLock) {
isTracingAppLaunch = NO;
}
}

return;
}
#endif // SENTRY_TARGET_PROFILING_SUPPORTED
Expand Down
14 changes: 0 additions & 14 deletions Sources/Sentry/include/SentryFileManager.h
Original file line number Diff line number Diff line change
Expand Up @@ -124,20 +124,6 @@ SENTRY_EXTERN void writeAppLaunchProfilingConfigFile(
*/
SENTRY_EXTERN void removeAppLaunchProfilingConfigFile(void);

/**
* Save a current launch profile config file for use when the associated transaction needs to be
* started. This is when checking @c SentryOptions for whether to write a config for the next
* launch; if there's no current app launch profile going, then we'd simply remove whatever config
* file may exist, but if there is a launch profile going, we need to save the config that it used.
*/
SENTRY_EXTERN void backupAppLaunchProfilingConfigFile(void);

/**
* Clean up the backup file saved off in the case that an app launch is in-flight when the SDK
* configures subsequent launches not to profile, requiring the current config to be saved for the
* associated transaction. This can be cleaned up when that transaction is started.
*/
SENTRY_EXTERN void removeAppLaunchProfilingConfigBackupFile(void);
#endif // SENTRY_TARGET_PROFILING_SUPPORTED

@end
Expand Down
3 changes: 0 additions & 3 deletions Sources/Sentry/include/SentryLaunchProfiling.h
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,6 @@
NS_ASSUME_NONNULL_BEGIN

SENTRY_EXTERN BOOL isTracingAppLaunch;
SENTRY_EXTERN SentryId *_Nullable appLaunchTraceId;
SENTRY_EXTERN uint64_t appLaunchSystemTime;
SENTRY_EXTERN NSObject *appLaunchTraceLock;

/** Try to start a profiled trace for this app launch, if the configuration allows. */
void startLaunchProfile(void);
Expand Down
36 changes: 0 additions & 36 deletions Tests/SentryTests/Helper/SentryFileManagerTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -795,42 +795,6 @@ extension SentryFileManagerTests {
removeAppLaunchProfilingConfigFile()
expect(NSDictionary(contentsOf: launchProfileConfigFileURL())) == nil
}

func testBackupAppLaunchProfilingConfigFile() throws {
try ensureAppLaunchProfileConfig(exists: true)
try ensureAppLaunchProfileConfig(exists: false, backup: true)
expect(NSDictionary(contentsOf: launchProfileConfigFileURL())) != nil
expect(NSDictionary(contentsOf: launchProfileConfigBackupFileURL())) == nil
backupAppLaunchProfilingConfigFile()
expect(NSDictionary(contentsOf: launchProfileConfigFileURL())) == nil
expect(NSDictionary(contentsOf: launchProfileConfigBackupFileURL())) != nil
}

// if a file is still present in the backup location, like if a crash occurred before it could be removed, or an error occurred when trying to remove it, make sure we overwrite it
func testBackupAppLaunchProfilingConfigFile_anotherBackupFilePresent() throws {
try ensureAppLaunchProfileConfig(exists: true, tracesSampleRate: 0.1, profilesSampleRate: 0.2)
try ensureAppLaunchProfileConfig(exists: true, tracesSampleRate: 0.3, profilesSampleRate: 0.4, backup: true)
expect(NSDictionary(contentsOf: launchProfileConfigFileURL())) != nil
expect(NSDictionary(contentsOf: launchProfileConfigBackupFileURL())) != nil
backupAppLaunchProfilingConfigFile()
expect(NSDictionary(contentsOf: launchProfileConfigFileURL())) == nil
expect(NSDictionary(contentsOf: launchProfileConfigBackupFileURL())) != nil
}

func testRemoveAppLaunchProfilingConfigBackupFile() throws {
try ensureAppLaunchProfileConfig(exists: true, backup: true)
expect(NSDictionary(contentsOf: launchProfileConfigBackupFileURL())) != nil
removeAppLaunchProfilingConfigBackupFile()
expect(NSDictionary(contentsOf: launchProfileConfigBackupFileURL())) == nil
}

// if there's not a file when we expect one, just make sure we don't crash
func testRemoveAppLaunchProfilingConfigBackupFile_noFileExists() throws {
try ensureAppLaunchProfileConfig(exists: false, backup: true)
expect(NSDictionary(contentsOf: launchProfileConfigBackupFileURL())) == nil
removeAppLaunchProfilingConfigBackupFile()
expect(NSDictionary(contentsOf: launchProfileConfigBackupFileURL())) == nil
}
}

// MARK: Private profiling tests
Expand Down

0 comments on commit b4dffe4

Please sign in to comment.