From 2bfe5bb7072448b9c4dbd36d5ce3bcf66474f820 Mon Sep 17 00:00:00 2001 From: Indragie Karunaratne Date: Mon, 15 May 2023 22:47:05 -0700 Subject: [PATCH 1/7] fix: additional profiling data races --- Sentry.xcodeproj/project.pbxproj | 4 + Sources/Sentry/SentryProfiler.mm | 378 ++++++++---------- .../Sentry/include/SentryProfiler+Private.h | 65 +++ Sources/Sentry/include/SentryProfiler+Test.h | 9 +- .../SentryProfilerTests.mm | 102 ++--- 5 files changed, 268 insertions(+), 290 deletions(-) create mode 100644 Sources/Sentry/include/SentryProfiler+Private.h diff --git a/Sentry.xcodeproj/project.pbxproj b/Sentry.xcodeproj/project.pbxproj index b1e12416145..53fbd5ca682 100644 --- a/Sentry.xcodeproj/project.pbxproj +++ b/Sentry.xcodeproj/project.pbxproj @@ -7,6 +7,7 @@ objects = { /* Begin PBXBuildFile section */ + 0354A22B2A134D9C003C3A04 /* SentryProfiler+Private.h in Headers */ = {isa = PBXBuildFile; fileRef = 0354A22A2A134D9C003C3A04 /* SentryProfiler+Private.h */; }; 0356A570288B4612008BF593 /* SentryProfilesSampler.h in Headers */ = {isa = PBXBuildFile; fileRef = 0356A56E288B4612008BF593 /* SentryProfilesSampler.h */; }; 0356A571288B4612008BF593 /* SentryProfilesSampler.m in Sources */ = {isa = PBXBuildFile; fileRef = 0356A56F288B4612008BF593 /* SentryProfilesSampler.m */; }; 03BCC38A27E1BF49003232C7 /* SentryTime.h in Headers */ = {isa = PBXBuildFile; fileRef = 03BCC38927E1BF49003232C7 /* SentryTime.h */; }; @@ -861,6 +862,7 @@ /* End PBXContainerItemProxy section */ /* Begin PBXFileReference section */ + 0354A22A2A134D9C003C3A04 /* SentryProfiler+Private.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = "SentryProfiler+Private.h"; path = "Sources/Sentry/include/SentryProfiler+Private.h"; sourceTree = SOURCE_ROOT; }; 0356A56E288B4612008BF593 /* SentryProfilesSampler.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; name = SentryProfilesSampler.h; path = Sources/Sentry/include/SentryProfilesSampler.h; sourceTree = SOURCE_ROOT; }; 0356A56F288B4612008BF593 /* SentryProfilesSampler.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; name = SentryProfilesSampler.m; path = Sources/Sentry/SentryProfilesSampler.m; sourceTree = SOURCE_ROOT; }; 035E73C727D56757005EEB11 /* SentryBacktraceTests.mm */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.objcpp; path = SentryBacktraceTests.mm; sourceTree = ""; }; @@ -3064,6 +3066,7 @@ 03F84D1127DD414C008FE43F /* SentryProfiler.h */, 844EDD6B2949387000C86F34 /* SentryMetricProfiler.h */, 8454CF8B293EAF9A006AC140 /* SentryMetricProfiler.mm */, + 0354A22A2A134D9C003C3A04 /* SentryProfiler+Private.h */, 84A888FC28D9B11700C51DFD /* SentryProfiler+Test.h */, 844EDC7B2942843400C86F34 /* SentryProfiler+SwiftTest.h */, 03F84D2B27DD4191008FE43F /* SentryProfiler.mm */, @@ -3515,6 +3518,7 @@ 7BC8522F24581096005A70F0 /* SentryFileContents.h in Headers */, 7B30B67C26527886006B2752 /* SentryDisplayLinkWrapper.h in Headers */, 7BD86ECF264A7C77005439DB /* SentryAppStartMeasurement.h in Headers */, + 0354A22B2A134D9C003C3A04 /* SentryProfiler+Private.h in Headers */, 7DC8310A2398283C0043DD9A /* SentryCrashIntegration.h in Headers */, 63FE718320DA4C1100CDBAE8 /* SentryCrashReportFixer.h in Headers */, 03F84D2027DD414C008FE43F /* SentryStackBounds.hpp in Headers */, diff --git a/Sources/Sentry/SentryProfiler.mm b/Sources/Sentry/SentryProfiler.mm index 5d001a2ac3e..20c215fac35 100644 --- a/Sources/Sentry/SentryProfiler.mm +++ b/Sources/Sentry/SentryProfiler.mm @@ -1,3 +1,4 @@ +#import "SentryProfiler+Private.h" #import "SentryProfiler+Test.h" #if SENTRY_TARGET_PROFILING_SUPPORTED @@ -49,7 +50,6 @@ # endif const int kSentryProfilerFrequencyHz = 101; -NSString *const kTestStringConst = @"test"; NSTimeInterval kSentryProfilerTimeoutInterval = 30; NSString *const kSentryProfilerSerializationKeySlowFrameRenders = @"slow_frame_renders"; @@ -79,108 +79,6 @@ return [symbolNSStr substringWithRange:[match rangeAtIndex:1]]; } -std::mutex _gDataStructureLock; - -void -processBacktrace(const Backtrace &backtrace, - NSMutableDictionary *threadMetadata, - NSMutableDictionary *queueMetadata, - NSMutableArray *samples, NSMutableArray *> *stacks, - NSMutableArray *> *frames, - NSMutableDictionary *frameIndexLookup, - NSMutableDictionary *stackIndexLookup) -{ - const auto stack = [NSMutableArray array]; - const auto framesToAdd = [NSMutableArray *> array]; - for (std::vector::size_type backtraceAddressIdx = 0; - backtraceAddressIdx < backtrace.addresses.size(); backtraceAddressIdx++) { - const auto instructionAddress - = sentry_formatHexAddressUInt64(backtrace.addresses[backtraceAddressIdx]); - - const auto frameIndex = frameIndexLookup[instructionAddress]; - if (frameIndex == nil) { - const auto frame = [NSMutableDictionary dictionary]; - frame[@"instruction_addr"] = instructionAddress; -# if defined(DEBUG) - const auto symbols - = backtrace_symbols(reinterpret_cast(backtrace.addresses.data()), - static_cast(backtrace.addresses.size())); - frame[@"function"] = parseBacktraceSymbolsFunctionName(symbols[backtraceAddressIdx]); -# endif - [stack addObject:@(frames.count)]; - frameIndexLookup[instructionAddress] = @(frames.count); - [framesToAdd addObject:frame]; - } else { - [stack addObject:frameIndex]; - } - } - - const auto sample = [[SentrySample alloc] init]; - sample.absoluteTimestamp = backtrace.absoluteTimestamp; - sample.threadID = backtrace.threadMetadata.threadID; - - NSString *queueAddress = nil; - if (backtrace.queueMetadata.address != 0) { - queueAddress = sentry_formatHexAddressUInt64(backtrace.queueMetadata.address); - } - if (queueAddress != nil) { - sample.queueAddress = queueAddress; - } - - const auto stackKey = [stack componentsJoinedByString:@"|"]; - const auto stackIndex = stackIndexLookup[stackKey]; - if (stackIndex) { - sample.stackIndex = stackIndex; - } else { - const auto nextStackIndex = @(stacks.count); - sample.stackIndex = nextStackIndex; - stackIndexLookup[stackKey] = nextStackIndex; - } - - { - // critical section: here is where all mutable data structures are updated, which must be - // copied later during serialization, so we need enforce exclusive access - std::lock_guard l(_gDataStructureLock); - - // update queue metadata if we have any and it isn't already recorded from a previous sample - if (queueAddress != nil && queueMetadata[queueAddress] == nil - && backtrace.queueMetadata.label != nullptr) { - queueMetadata[queueAddress] = @ { - @"label" : [NSString stringWithUTF8String:backtrace.queueMetadata.label->c_str()] - }; - } - - // The following can overwrite thread metadata that was previously recorded for a thread, - // when between samples if we find that the metadata is not present on one sample but is - // present for a subsequent sample on the same thread. This occasionally happens when we - // can't read the metadata for a thread (unclear why this happens but it tends to happen - // sometimes for a newly created thread). - const auto threadID = sentry_stringForUInt64(backtrace.threadMetadata.threadID); - NSMutableDictionary *metadata = threadMetadata[threadID]; - if (metadata == nil) { - metadata = [NSMutableDictionary dictionary]; - threadMetadata[threadID] = metadata; - } - if (!backtrace.threadMetadata.name.empty() && metadata[@"name"] == nil) { - metadata[@"name"] = - [NSString stringWithUTF8String:backtrace.threadMetadata.name.c_str()]; - } - if (backtrace.threadMetadata.priority != -1 && metadata[@"priority"] == nil) { - metadata[@"priority"] = @(backtrace.threadMetadata.priority); - } - - // add the new sample to the data structure - [samples addObject:sample]; - - // add the stack if it isn't already cached in the lookup index - if (stackIndex == nil) { - [stacks addObject:stack]; - } - - [frames addObjectsFromArray:framesToAdd]; - } -} - std::mutex _gProfilerLock; SentryProfiler *_Nullable _gCurrentProfiler; SentryNSProcessInfoWrapper *_gCurrentProcessInfoWrapper; @@ -290,42 +188,25 @@ SentryId *profileID, NSString *truncationReason, NSString *environment, NSString *release, NSDictionary *serializedMetrics, NSArray *debugMeta) { - NSArray *samplesCopy; - NSArray *> *stacksCopy; - NSArray *> *framesCopy; - NSMutableDictionary *threadMetadataCopy; - NSDictionary *queueMetadataCopy; - { - std::lock_guard d(_gDataStructureLock); - samplesCopy = [profileData[@"profile"][@"samples"] copy]; - stacksCopy = [profileData[@"profile"][@"stacks"] copy]; - framesCopy = [profileData[@"profile"][@"frames"] copy]; - queueMetadataCopy = [profileData[@"profile"][@"queue_metadata"] copy]; - - // thread metadata contains a mutable substructure, so it's not enough to perform a copy of - // the top-level dictionary, we need to go deeper to copy the mutable subdictionaries - threadMetadataCopy = [NSMutableDictionary dictionary]; - [((NSDictionary *)profileData[@"profile"][@"thread_metadata"]) - enumerateKeysAndObjectsUsingBlock:^(NSString *_Nonnull key, NSDictionary *_Nonnull obj, - BOOL *_Nonnull stop) { threadMetadataCopy[key] = [obj copy]; }]; - } - + NSMutableArray *const samples = profileData[@"profile"][@"samples"]; // We need at least two samples to be able to draw a stack frame for any given function: one // sample for the start of the frame and another for the end. Otherwise we would only have a // stack frame with 0 duration, which wouldn't make sense. - if ([samplesCopy count] < 2) { + if ([samples count] < 2) { SENTRY_LOG_DEBUG(@"Not enough samples in profile"); return nil; } // slice the profile data to only include the samples/metrics within the transaction - const auto slicedSamples = slicedProfileSamples(samplesCopy, transaction); + const auto slicedSamples = slicedProfileSamples(samples, transaction); if (slicedSamples.count < 2) { SENTRY_LOG_DEBUG(@"Not enough samples in profile during the transaction"); return nil; } - const auto payload = [NSMutableDictionary dictionary]; + NSMutableDictionary *const profile = [profileData[@"profile"] mutableCopy]; + profile[@"samples"] = serializedSamplesWithRelativeTimestamps(slicedSamples, transaction); + payload[@"profile"] = profile; payload[@"version"] = @"1"; const auto debugImages = [NSMutableArray *> new]; @@ -366,15 +247,6 @@ } payload[@"release"] = release; - - payload[@"profile"] = @ { - @"samples" : serializedSamplesWithRelativeTimestamps(slicedSamples, transaction), - @"stacks" : stacksCopy, - @"frames" : framesCopy, - @"thread_metadata" : threadMetadataCopy, - @"queue_metadata" : queueMetadataCopy, - }; - payload[@"transaction"] = @ { @"id" : transaction.eventId.sentryIdString, @"trace_id" : transaction.trace.traceId.sentryIdString, @@ -421,12 +293,150 @@ return payload; } +@implementation SentryProfilingMutableState + +- (instancetype)init { + if (self = [super init]) { + _samples = [NSMutableArray array]; + _stacks = [NSMutableArray *> array]; + _frames = [NSMutableArray *> array]; + _threadMetadata = [NSMutableDictionary dictionary]; + _queueMetadata = [NSMutableDictionary dictionary]; + _frameIndexLookup = [NSMutableDictionary dictionary]; + _stackIndexLookup = [NSMutableDictionary dictionary]; + } + return self; +} + +@end + +@implementation SentryProfilingState { + SentryProfilingMutableState *_mutableState; + std::mutex _lock; +} + +- (instancetype)init +{ + if (self = [super init]) { + _mutableState = [[SentryProfilingMutableState alloc] init]; + } + return self; +} + +- (void)mutate:(void (^)(SentryProfilingMutableState *))block +{ + NSParameterAssert(block); + std::lock_guard l(_lock); + block(_mutableState); +} + +- (void)appendBacktrace:(const Backtrace &)backtrace { + [self mutate:^(SentryProfilingMutableState *state) { + const auto threadID = sentry_stringForUInt64(backtrace.threadMetadata.threadID); + + NSString *queueAddress = nil; + if (backtrace.queueMetadata.address != 0) { + queueAddress = sentry_formatHexAddressUInt64(backtrace.queueMetadata.address); + } + NSMutableDictionary *metadata = state.threadMetadata[threadID]; + if (metadata == nil) { + metadata = [NSMutableDictionary dictionary]; + state.threadMetadata[threadID] = metadata; + } + if (!backtrace.threadMetadata.name.empty() && metadata[@"name"] == nil) { + metadata[@"name"] = [NSString stringWithUTF8String:backtrace.threadMetadata.name.c_str()]; + } + if (backtrace.threadMetadata.priority != -1 && metadata[@"priority"] == nil) { + metadata[@"priority"] = @(backtrace.threadMetadata.priority); + } + if (queueAddress != nil && state.queueMetadata[queueAddress] == nil + && backtrace.queueMetadata.label != nullptr) { + state.queueMetadata[queueAddress] = + @ { @"label" : [NSString stringWithUTF8String:backtrace.queueMetadata.label->c_str()] }; + } + # if defined(DEBUG) + const auto symbols + = backtrace_symbols(reinterpret_cast(backtrace.addresses.data()), + static_cast(backtrace.addresses.size())); + # endif + + const auto stack = [NSMutableArray array]; + for (std::vector::size_type backtraceAddressIdx = 0; + backtraceAddressIdx < backtrace.addresses.size(); backtraceAddressIdx++) { + const auto instructionAddress + = sentry_formatHexAddressUInt64(backtrace.addresses[backtraceAddressIdx]); + + const auto frameIndex = state.frameIndexLookup[instructionAddress]; + if (frameIndex == nil) { + const auto frame = [NSMutableDictionary dictionary]; + frame[@"instruction_addr"] = instructionAddress; + # if defined(DEBUG) + frame[@"function"] = parseBacktraceSymbolsFunctionName(symbols[backtraceAddressIdx]); + # endif + const auto newFrameIndex = @(state.frames.count); + [stack addObject:newFrameIndex]; + state.frameIndexLookup[instructionAddress] = newFrameIndex; + [state.frames addObject:frame]; + } else { + [stack addObject:frameIndex]; + } + } + + const auto sample = [[SentrySample alloc] init]; + sample.absoluteTimestamp = backtrace.absoluteTimestamp; + sample.threadID = backtrace.threadMetadata.threadID; + if (queueAddress != nil) { + sample.queueAddress = queueAddress; + } + + const auto stackKey = [stack componentsJoinedByString:@"|"]; + const auto stackIndex = state.stackIndexLookup[stackKey]; + if (stackIndex) { + sample.stackIndex = stackIndex; + } else { + const auto nextStackIndex = @(state.stacks.count); + sample.stackIndex = nextStackIndex; + state.stackIndexLookup[stackKey] = nextStackIndex; + [state.stacks addObject:stack]; + } + + [state.samples addObject:sample]; + }]; +} + +- (NSDictionary *)copyProfilingData { + std::lock_guard l(_lock); + + NSMutableArray *const samples = [_mutableState.samples copy]; + NSMutableArray *> *const stacks = [_mutableState.stacks copy]; + NSMutableArray *> *const frames = [_mutableState.frames copy]; + NSMutableDictionary *const queueMetadata = [_mutableState.queueMetadata copy]; + + // thread metadata contains a mutable substructure, so it's not enough to perform a copy of + // the top-level dictionary, we need to go deeper to copy the mutable subdictionaries + const auto threadMetadata = [NSMutableDictionary dictionary]; + [_mutableState.threadMetadata + enumerateKeysAndObjectsUsingBlock:^(NSString *_Nonnull key, NSDictionary *_Nonnull obj, + BOOL *_Nonnull stop) { threadMetadata[key] = [obj copy]; }]; + + return @{ + @"profile": @{ + @"samples": samples, + @"stacks": stacks, + @"frames": frames, + @"thread_metadata": threadMetadata, + @"queue_metadata": queueMetadata + } + }; +} + +@end + @implementation SentryProfiler { - NSMutableDictionary *_profileData; + SentryProfilingState *_state; std::shared_ptr _profiler; SentryMetricProfiler *_metricProfiler; SentryDebugImageProvider *_debugImageProvider; - thread::TIDType _mainThreadID; SentryProfilerTruncationReason _truncationReason; NSTimer *_timeoutTimer; @@ -442,7 +452,6 @@ - (instancetype)initWithHub:(SentryHub *)hub SENTRY_LOG_DEBUG(@"Initialized new SentryProfiler %@", self); _debugImageProvider = [SentryDependencyContainer sharedInstance].debugImageProvider; _hub = hub; - _mainThreadID = ThreadHandle::current()->tid(); return self; } @@ -568,7 +577,7 @@ + (void)useFramesTracker:(SentryFramesTracker *)framesTracker return nil; } - return serializedProfileData(_gCurrentProfiler->_profileData, transaction, profileID, + return serializedProfileData([_gCurrentProfiler->_state copyProfilingData], transaction, profileID, profilerTruncationReasonName(_gCurrentProfiler->_truncationReason), _gCurrentProfiler->_hub.scope.environmentString ?: _gCurrentProfiler->_hub.getClient.options.environment, @@ -678,83 +687,24 @@ - (void)start SENTRY_LOG_DEBUG(@"Starting profiler."); - _profileData = [NSMutableDictionary dictionary]; - const auto sampledProfile = [NSMutableDictionary dictionary]; - - /* - * Maintain an index of unique frames to avoid duplicating large amounts of data. Every - * unique frame is stored in an array, and every time a stack trace is captured for a - * sample, the stack is stored as an array of integers indexing into the array of frames. - * Stacks are thusly also stored as unique elements in their own index, an array of arrays - * of frame indices, and each sample references a stack by index, to deduplicate common - * stacks between samples, such as when the same deep function call runs across multiple - * samples. - * - * E.g. if we have the following samples in the following function call stacks: - * - * v sample1 v sample2 v sample3 v sample4 - * |-foo--------|------------|-----| |-abc--------|------------|-----| - * |-bar-----|------------|--| |-def-----|------------|--| - * |-baz---|------------|-| |-ghi---|------------|-| - * - * Then we'd wind up with the following structures: - * - * frames: [ - * { function: foo, instruction_addr: ... }, - * { function: bar, instruction_addr: ... }, - * { function: baz, instruction_addr: ... }, - * { function: abc, instruction_addr: ... }, - * { function: def, instruction_addr: ... }, - * { function: ghi, instruction_addr: ... } - * ] - * stacks: [ [0, 1, 2], [3, 4, 5] ] - * samples: [ - * { stack_id: 0, ... }, - * { stack_id: 0, ... }, - * { stack_id: 1, ... }, - * { stack_id: 1, ... } - * ] - */ - const auto samples = [NSMutableArray array]; - const auto stacks = [NSMutableArray *> array]; - const auto frames = [NSMutableArray *> array]; - const auto frameIndexLookup = [NSMutableDictionary dictionary]; - const auto stackIndexLookup = [NSMutableDictionary dictionary]; - sampledProfile[@"samples"] = samples; - sampledProfile[@"stacks"] = stacks; - sampledProfile[@"frames"] = frames; - - const auto threadMetadata = [NSMutableDictionary dictionary]; - const auto queueMetadata = [NSMutableDictionary dictionary]; - sampledProfile[@"thread_metadata"] = threadMetadata; - sampledProfile[@"queue_metadata"] = queueMetadata; - _profileData[@"profile"] = sampledProfile; - - __weak const auto weakSelf = self; - _profiler = std::make_shared( - [weakSelf, threadMetadata, queueMetadata, samples, mainThreadID = _mainThreadID, frames, - frameIndexLookup, stacks, stackIndexLookup](auto &backtrace) { - const auto strongSelf = weakSelf; - if (strongSelf == nil) { - SENTRY_LOG_WARN(@"Profiler instance no longer exists, cannot process next sample."); - return; - } - - // in test, we'll overwrite the sample's timestamp to one mocked by SentryCurrentDate etal. - // Doing this in a unified way between tests and production required extensive changes to - // the C++ layer, so we opted for this solution to avoid any potential breakages or - // performance hits there. -# if defined(TEST) || defined(TESTCI) + SentryProfilingState *const state = [[SentryProfilingState alloc] init]; + _state = state; + _profiler = std::make_shared([state](auto &backtrace) { + @autoreleasepool { + // in test, we'll overwrite the sample's timestamp to one mocked by SentryCurrentDate etal. + // Doing this in a unified way between tests and production required extensive changes to + // the C++ layer, so we opted for this solution to avoid any potential breakages or + // performance hits there. + # if defined(TEST) || defined(TESTCI) Backtrace backtraceCopy = backtrace; backtraceCopy.absoluteTimestamp = SentryCurrentDate.systemTime; - processBacktrace(backtraceCopy, threadMetadata, queueMetadata, samples, stacks, frames, - frameIndexLookup, stackIndexLookup); -# else - processBacktrace(backtrace, threadMetadata, queueMetadata, samples, stacks, frames, - frameIndexLookup, stackIndexLookup); -# endif // defined(TEST) || defined(TESTCI) - }, - kSentryProfilerFrequencyHz); + [state appendBacktrace:backtraceCopy]; + # else + [state appendBacktrace:backtrace]; + # endif // defined(TEST) || defined(TESTCI) + } + }, + kSentryProfilerFrequencyHz); _profiler->startSampling(); [self startMetricProfiler]; diff --git a/Sources/Sentry/include/SentryProfiler+Private.h b/Sources/Sentry/include/SentryProfiler+Private.h new file mode 100644 index 00000000000..9b39e7d899f --- /dev/null +++ b/Sources/Sentry/include/SentryProfiler+Private.h @@ -0,0 +1,65 @@ +#import +#import "SentryBacktrace.hpp" +#import "SentryProfilingConditionals.h" + +#if SENTRY_TARGET_PROFILING_SUPPORTED + +NS_ASSUME_NONNULL_BEGIN + +@class SentrySample; + +@interface SentryProfilingMutableState : NSObject +@property (nonatomic, strong, readonly) NSMutableArray *samples; +@property (nonatomic, strong, readonly) NSMutableArray *> *stacks; +@property (nonatomic, strong, readonly) NSMutableArray *> *frames; +@property (nonatomic, strong, readonly) NSMutableDictionary *threadMetadata; +@property (nonatomic, strong, readonly) NSMutableDictionary *queueMetadata; + +/* + * Maintain an index of unique frames to avoid duplicating large amounts of data. Every + * unique frame is stored in an array, and every time a stack trace is captured for a + * sample, the stack is stored as an array of integers indexing into the array of frames. + * Stacks are thusly also stored as unique elements in their own index, an array of arrays + * of frame indices, and each sample references a stack by index, to deduplicate common + * stacks between samples, such as when the same deep function call runs across multiple + * samples. + * + * E.g. if we have the following samples in the following function call stacks: + * + * v sample1 v sample2 v sample3 v sample4 + * |-foo--------|------------|-----| |-abc--------|------------|-----| + * |-bar-----|------------|--| |-def-----|------------|--| + * |-baz---|------------|-| |-ghi---|------------|-| + * + * Then we'd wind up with the following structures: + * + * frames: [ + * { function: foo, instruction_addr: ... }, + * { function: bar, instruction_addr: ... }, + * { function: baz, instruction_addr: ... }, + * { function: abc, instruction_addr: ... }, + * { function: def, instruction_addr: ... }, + * { function: ghi, instruction_addr: ... } + * ] + * stacks: [ [0, 1, 2], [3, 4, 5] ] + * samples: [ + * { stack_id: 0, ... }, + * { stack_id: 0, ... }, + * { stack_id: 1, ... }, + * { stack_id: 1, ... } + * ] + */ +@property (nonatomic, strong, readonly) NSMutableDictionary *frameIndexLookup; +@property (nonatomic, strong, readonly) NSMutableDictionary *stackIndexLookup; +@end + +@interface SentryProfilingState : NSObject +// All functions are safe to call from multiple threads concurrently +- (void)mutate:(void (^)(SentryProfilingMutableState *))block; +- (void)appendBacktrace:(const sentry::profiling::Backtrace &)backtrace; +- (NSDictionary *)copyProfilingData; +@end + +NS_ASSUME_NONNULL_END + +#endif diff --git a/Sources/Sentry/include/SentryProfiler+Test.h b/Sources/Sentry/include/SentryProfiler+Test.h index d8bb5f233ed..580ad78a79d 100644 --- a/Sources/Sentry/include/SentryProfiler+Test.h +++ b/Sources/Sentry/include/SentryProfiler+Test.h @@ -1,5 +1,6 @@ #include "SentryBacktrace.hpp" #import "SentryProfiler.h" +#import "SentryProfiler+Private.h" #import "SentryProfilingConditionals.h" #if SENTRY_TARGET_PROFILING_SUPPORTED @@ -11,14 +12,6 @@ NS_ASSUME_NONNULL_BEGIN -void processBacktrace(const sentry::profiling::Backtrace &backtrace, - NSMutableDictionary *threadMetadata, - NSMutableDictionary *queueMetadata, - NSMutableArray *samples, NSMutableArray *> *stacks, - NSMutableArray *> *frames, - NSMutableDictionary *frameIndexLookup, - NSMutableDictionary *stackIndexLookup); - NSDictionary *serializedProfileData(NSDictionary *profileData, SentryTransaction *transaction, SentryId *profileID, NSString *truncationReason, NSString *environment, NSString *release, NSDictionary *serializedMetrics, diff --git a/Tests/SentryProfilerTests/SentryProfilerTests.mm b/Tests/SentryProfilerTests/SentryProfilerTests.mm index 052bfa1665c..0ae529d5f75 100644 --- a/Tests/SentryProfilerTests/SentryProfilerTests.mm +++ b/Tests/SentryProfilerTests/SentryProfilerTests.mm @@ -60,15 +60,7 @@ - (void)testProfilerCanBeInitializedOffMainThread - (void)testProfilerMutationDuringSlicing { - const auto resolvedThreadMetadata = - [NSMutableDictionary dictionary]; - const auto resolvedQueueMetadata = [NSMutableDictionary dictionary]; - const auto resolvedStacks = [NSMutableArray *> array]; - const auto resolvedSamples = [NSMutableArray array]; - const auto resolvedFrames = [NSMutableArray *> array]; - const auto frameIndexLookup = [NSMutableDictionary dictionary]; - const auto stackIndexLookup = [NSMutableDictionary dictionary]; - + SentryProfilingState *state = [[SentryProfilingState alloc] init]; // generate a large timeseries of simulated data const auto threads = 5; @@ -100,9 +92,7 @@ - (void)testProfilerMutationDuringSlicing for (auto sample = 0; sample < samplesPerThread; sample++) { backtrace.absoluteTimestamp = sampleIdx; // simulate 1 sample per nanosecond - processBacktrace(backtrace, resolvedThreadMetadata, resolvedQueueMetadata, - resolvedSamples, resolvedStacks, resolvedFrames, frameIndexLookup, - stackIndexLookup); + [state appendBacktrace:backtrace]; ++sampleIdx; } } @@ -127,8 +117,10 @@ - (void)testProfilerMutationDuringSlicing sliceExpectation.expectedFulfillmentCount = operations; void (^sliceBlock)(void) = ^(void) { - __unused const auto slice = slicedProfileSamples(resolvedSamples, transaction); - [sliceExpectation fulfill]; + [state mutate:^(SentryProfilingMutableState *mutableState) { + __unused const auto slice = slicedProfileSamples(mutableState.samples, transaction); + [sliceExpectation fulfill]; + }]; }; ThreadMetadata threadMetadata; @@ -153,9 +145,9 @@ - (void)testProfilerMutationDuringSlicing mutateExpectation.expectedFulfillmentCount = operations; void (^mutateBlock)(void) = ^(void) { - processBacktrace(backtrace, resolvedThreadMetadata, resolvedQueueMetadata, resolvedSamples, - resolvedStacks, resolvedFrames, frameIndexLookup, stackIndexLookup); - [mutateExpectation fulfill]; + [state mutate:^(__unused SentryProfilingMutableState *mutableState) { + [mutateExpectation fulfill]; + }]; }; const auto sliceOperations = [[NSOperationQueue alloc] init]; // concurrent queue @@ -184,15 +176,7 @@ - (void)testProfilerMutationDuringSlicing */ - (void)testProfilerMutationDuringSerialization { - const auto resolvedThreadMetadata = - [NSMutableDictionary dictionary]; - const auto resolvedQueueMetadata = [NSMutableDictionary dictionary]; - const auto resolvedStacks = [NSMutableArray *> array]; - const auto resolvedSamples = [NSMutableArray array]; - const auto resolvedFrames = [NSMutableArray *> array]; - const auto frameIndexLookup = [NSMutableDictionary dictionary]; - const auto stackIndexLookup = [NSMutableDictionary dictionary]; - + SentryProfilingState *state = [[SentryProfilingState alloc] init]; // initialize the data structures with some simulated data { ThreadMetadata threadMetadata; @@ -210,22 +194,14 @@ - (void)testProfilerMutationDuringSerialization backtrace.addresses = std::vector({ 0x4, 0x5, 0x6 }); backtrace.absoluteTimestamp = 1; - processBacktrace(backtrace, resolvedThreadMetadata, resolvedQueueMetadata, resolvedSamples, - resolvedStacks, resolvedFrames, frameIndexLookup, stackIndexLookup); - + [state appendBacktrace:backtrace]; + backtrace.absoluteTimestamp = 2; - processBacktrace(backtrace, resolvedThreadMetadata, resolvedQueueMetadata, resolvedSamples, - resolvedStacks, resolvedFrames, frameIndexLookup, stackIndexLookup); + [state appendBacktrace:backtrace]; } // serialize the data as if it were captured in a transaction envelope - const auto sampledProfile = [NSMutableDictionary dictionary]; - sampledProfile[@"samples"] = resolvedSamples; - sampledProfile[@"stacks"] = resolvedStacks; - sampledProfile[@"frames"] = resolvedFrames; - sampledProfile[@"thread_metadata"] = resolvedThreadMetadata; - sampledProfile[@"queue_metadata"] = resolvedQueueMetadata; - const auto profileData = @{ @"profile" : sampledProfile }; + const auto profileData = [state copyProfilingData]; const auto context = [[SentrySpanContext alloc] initWithOperation:@"test trace"]; const auto trace = [[SentryTracer alloc] initWithContext:context]; @@ -260,8 +236,7 @@ - (void)testProfilerMutationDuringSerialization backtrace.absoluteTimestamp = 5; backtrace.addresses = std::vector({ 0x777, 0x888, 0x999 }); - processBacktrace(backtrace, resolvedThreadMetadata, resolvedQueueMetadata, resolvedSamples, - resolvedStacks, resolvedFrames, frameIndexLookup, stackIndexLookup); + [state appendBacktrace:backtrace]; } // cause the data structures to be modified again: overwrite previous thread metadata @@ -282,8 +257,7 @@ - (void)testProfilerMutationDuringSerialization backtrace.absoluteTimestamp = 6; backtrace.addresses = std::vector({ 0x4, 0x5, 0x6 }); - processBacktrace(backtrace, resolvedThreadMetadata, resolvedQueueMetadata, resolvedSamples, - resolvedStacks, resolvedFrames, frameIndexLookup, stackIndexLookup); + [state appendBacktrace:backtrace]; } // ensure the serialization's copied data structures don't contain the new addresses @@ -314,14 +288,7 @@ - (void)testProfilerMutationDuringSerialization - (void)testProfilerPayload { - const auto resolvedThreadMetadata = - [NSMutableDictionary dictionary]; - const auto resolvedQueueMetadata = [NSMutableDictionary dictionary]; - const auto resolvedStacks = [NSMutableArray *> array]; - const auto resolvedSamples = [NSMutableArray array]; - const auto resolvedFrames = [NSMutableArray *> array]; - const auto frameIndexLookup = [NSMutableDictionary dictionary]; - const auto stackIndexLookup = [NSMutableDictionary dictionary]; + SentryProfilingState *state = [[SentryProfilingState alloc] init]; // record an initial backtrace @@ -342,8 +309,7 @@ - (void)testProfilerPayload backtrace1.absoluteTimestamp = 5; backtrace1.addresses = addresses1; - processBacktrace(backtrace1, resolvedThreadMetadata, resolvedQueueMetadata, resolvedSamples, - resolvedStacks, resolvedFrames, frameIndexLookup, stackIndexLookup); + [state appendBacktrace:backtrace1]; // record a second backtrace with some common addresses to test frame deduplication @@ -364,26 +330,26 @@ - (void)testProfilerPayload backtrace2.absoluteTimestamp = 5; backtrace2.addresses = addresses2; - processBacktrace(backtrace2, resolvedThreadMetadata, resolvedQueueMetadata, resolvedSamples, - resolvedStacks, resolvedFrames, frameIndexLookup, stackIndexLookup); + [state appendBacktrace:backtrace2]; // record a third backtrace that's identical to the second to test stack/frame deduplication - processBacktrace(backtrace2, resolvedThreadMetadata, resolvedQueueMetadata, resolvedSamples, - resolvedStacks, resolvedFrames, frameIndexLookup, stackIndexLookup); - - XCTAssertEqual(resolvedFrames.count, 5UL); - const auto expectedOrdered = @[ - @"0x0000000000000123", @"0x0000000000000456", @"0x0000000000000789", @"0x0000000000000777", - @"0x0000000000000888" - ]; - [resolvedFrames enumerateObjectsUsingBlock:^(NSDictionary *_Nonnull actualFrame, - NSUInteger idx, __unused BOOL *_Nonnull stop) { - XCTAssert([actualFrame[@"instruction_addr"] isEqualToString:expectedOrdered[idx]]); + [state appendBacktrace:backtrace2]; + + [state mutate:^(SentryProfilingMutableState *mutableState) { + XCTAssertEqual(mutableState.frames.count, 5UL); + const auto expectedOrdered = @[ + @"0x0000000000000123", @"0x0000000000000456", @"0x0000000000000789", @"0x0000000000000777", + @"0x0000000000000888" + ]; + [mutableState.frames enumerateObjectsUsingBlock:^(NSDictionary *_Nonnull actualFrame, + NSUInteger idx, __unused BOOL *_Nonnull stop) { + XCTAssert([actualFrame[@"instruction_addr"] isEqualToString:expectedOrdered[idx]]); + }]; + + XCTAssertEqual(mutableState.stacks.count, 2UL); + XCTAssertEqual(mutableState.samples.count, 3UL); }]; - - XCTAssertEqual(resolvedStacks.count, 2UL); - XCTAssertEqual(resolvedSamples.count, 3UL); } @end From 23129ead3841b191441e068c856e79574d72a864 Mon Sep 17 00:00:00 2001 From: Indragie Karunaratne Date: Mon, 15 May 2023 22:50:45 -0700 Subject: [PATCH 2/7] Remove unused kTestStringConst --- Sources/Sentry/include/SentryProfiler.h | 1 - 1 file changed, 1 deletion(-) diff --git a/Sources/Sentry/include/SentryProfiler.h b/Sources/Sentry/include/SentryProfiler.h index 2bfa6d16f26..2fa99f8331c 100644 --- a/Sources/Sentry/include/SentryProfiler.h +++ b/Sources/Sentry/include/SentryProfiler.h @@ -21,7 +21,6 @@ typedef NS_ENUM(NSUInteger, SentryProfilerTruncationReason) { NS_ASSUME_NONNULL_BEGIN SENTRY_EXTERN const int kSentryProfilerFrequencyHz; -SENTRY_EXTERN NSString *const kTestStringConst; SENTRY_EXTERN NSString *const kSentryProfilerSerializationKeySlowFrameRenders; SENTRY_EXTERN NSString *const kSentryProfilerSerializationKeyFrozenFrameRenders; From 0f264577757f0f2328b667e66a62531daa168fe6 Mon Sep 17 00:00:00 2001 From: Sentry Github Bot Date: Tue, 16 May 2023 06:09:46 +0000 Subject: [PATCH 3/7] Format code --- Sources/Sentry/SentryProfiler.mm | 97 ++++++++++--------- .../Sentry/include/SentryProfiler+Private.h | 14 ++- Sources/Sentry/include/SentryProfiler+Test.h | 2 +- .../SentryProfilerTests.mm | 24 ++--- 4 files changed, 74 insertions(+), 63 deletions(-) diff --git a/Sources/Sentry/SentryProfiler.mm b/Sources/Sentry/SentryProfiler.mm index 20c215fac35..7bc033dd24a 100644 --- a/Sources/Sentry/SentryProfiler.mm +++ b/Sources/Sentry/SentryProfiler.mm @@ -295,7 +295,8 @@ @implementation SentryProfilingMutableState -- (instancetype)init { +- (instancetype)init +{ if (self = [super init]) { _samples = [NSMutableArray array]; _stacks = [NSMutableArray *> array]; @@ -330,10 +331,11 @@ - (void)mutate:(void (^)(SentryProfilingMutableState *))block block(_mutableState); } -- (void)appendBacktrace:(const Backtrace &)backtrace { +- (void)appendBacktrace:(const Backtrace &)backtrace +{ [self mutate:^(SentryProfilingMutableState *state) { const auto threadID = sentry_stringForUInt64(backtrace.threadMetadata.threadID); - + NSString *queueAddress = nil; if (backtrace.queueMetadata.address != 0) { queueAddress = sentry_formatHexAddressUInt64(backtrace.queueMetadata.address); @@ -344,21 +346,23 @@ - (void)appendBacktrace:(const Backtrace &)backtrace { state.threadMetadata[threadID] = metadata; } if (!backtrace.threadMetadata.name.empty() && metadata[@"name"] == nil) { - metadata[@"name"] = [NSString stringWithUTF8String:backtrace.threadMetadata.name.c_str()]; + metadata[@"name"] = + [NSString stringWithUTF8String:backtrace.threadMetadata.name.c_str()]; } if (backtrace.threadMetadata.priority != -1 && metadata[@"priority"] == nil) { metadata[@"priority"] = @(backtrace.threadMetadata.priority); } if (queueAddress != nil && state.queueMetadata[queueAddress] == nil && backtrace.queueMetadata.label != nullptr) { - state.queueMetadata[queueAddress] = - @ { @"label" : [NSString stringWithUTF8String:backtrace.queueMetadata.label->c_str()] }; + state.queueMetadata[queueAddress] = @ { + @"label" : [NSString stringWithUTF8String:backtrace.queueMetadata.label->c_str()] + }; } - # if defined(DEBUG) +# if defined(DEBUG) const auto symbols = backtrace_symbols(reinterpret_cast(backtrace.addresses.data()), static_cast(backtrace.addresses.size())); - # endif +# endif const auto stack = [NSMutableArray array]; for (std::vector::size_type backtraceAddressIdx = 0; @@ -370,9 +374,10 @@ - (void)appendBacktrace:(const Backtrace &)backtrace { if (frameIndex == nil) { const auto frame = [NSMutableDictionary dictionary]; frame[@"instruction_addr"] = instructionAddress; - # if defined(DEBUG) - frame[@"function"] = parseBacktraceSymbolsFunctionName(symbols[backtraceAddressIdx]); - # endif +# if defined(DEBUG) + frame[@"function"] + = parseBacktraceSymbolsFunctionName(symbols[backtraceAddressIdx]); +# endif const auto newFrameIndex = @(state.frames.count); [stack addObject:newFrameIndex]; state.frameIndexLookup[instructionAddress] = newFrameIndex; @@ -388,7 +393,7 @@ - (void)appendBacktrace:(const Backtrace &)backtrace { if (queueAddress != nil) { sample.queueAddress = queueAddress; } - + const auto stackKey = [stack componentsJoinedByString:@"|"]; const auto stackIndex = state.stackIndexLookup[stackKey]; if (stackIndex) { @@ -399,33 +404,34 @@ - (void)appendBacktrace:(const Backtrace &)backtrace { state.stackIndexLookup[stackKey] = nextStackIndex; [state.stacks addObject:stack]; } - + [state.samples addObject:sample]; }]; } -- (NSDictionary *)copyProfilingData { +- (NSDictionary *)copyProfilingData +{ std::lock_guard l(_lock); - + NSMutableArray *const samples = [_mutableState.samples copy]; NSMutableArray *> *const stacks = [_mutableState.stacks copy]; NSMutableArray *> *const frames = [_mutableState.frames copy]; - NSMutableDictionary *const queueMetadata = [_mutableState.queueMetadata copy]; + NSMutableDictionary *const queueMetadata = + [_mutableState.queueMetadata copy]; // thread metadata contains a mutable substructure, so it's not enough to perform a copy of // the top-level dictionary, we need to go deeper to copy the mutable subdictionaries const auto threadMetadata = [NSMutableDictionary dictionary]; - [_mutableState.threadMetadata - enumerateKeysAndObjectsUsingBlock:^(NSString *_Nonnull key, NSDictionary *_Nonnull obj, - BOOL *_Nonnull stop) { threadMetadata[key] = [obj copy]; }]; - + [_mutableState.threadMetadata enumerateKeysAndObjectsUsingBlock:^(NSString *_Nonnull key, + NSDictionary *_Nonnull obj, BOOL *_Nonnull stop) { threadMetadata[key] = [obj copy]; }]; + return @{ - @"profile": @{ - @"samples": samples, - @"stacks": stacks, - @"frames": frames, - @"thread_metadata": threadMetadata, - @"queue_metadata": queueMetadata + @"profile" : @ { + @"samples" : samples, + @"stacks" : stacks, + @"frames" : frames, + @"thread_metadata" : threadMetadata, + @"queue_metadata" : queueMetadata } }; } @@ -577,9 +583,9 @@ + (void)useFramesTracker:(SentryFramesTracker *)framesTracker return nil; } - return serializedProfileData([_gCurrentProfiler->_state copyProfilingData], transaction, profileID, - profilerTruncationReasonName(_gCurrentProfiler->_truncationReason), - _gCurrentProfiler->_hub.scope.environmentString + return serializedProfileData([_gCurrentProfiler->_state copyProfilingData], transaction, + profileID, profilerTruncationReasonName(_gCurrentProfiler->_truncationReason), + _gCurrentProfiler -> _hub.scope.environmentString ?: _gCurrentProfiler->_hub.getClient.options.environment, _gCurrentProfiler->_hub.getClient.options.releaseName, [_gCurrentProfiler->_metricProfiler serializeForTransaction:transaction], @@ -689,22 +695,23 @@ - (void)start SentryProfilingState *const state = [[SentryProfilingState alloc] init]; _state = state; - _profiler = std::make_shared([state](auto &backtrace) { - @autoreleasepool { - // in test, we'll overwrite the sample's timestamp to one mocked by SentryCurrentDate etal. - // Doing this in a unified way between tests and production required extensive changes to - // the C++ layer, so we opted for this solution to avoid any potential breakages or - // performance hits there. - # if defined(TEST) || defined(TESTCI) - Backtrace backtraceCopy = backtrace; - backtraceCopy.absoluteTimestamp = SentryCurrentDate.systemTime; - [state appendBacktrace:backtraceCopy]; - # else - [state appendBacktrace:backtrace]; - # endif // defined(TEST) || defined(TESTCI) - } - }, - kSentryProfilerFrequencyHz); + _profiler = std::make_shared( + [state](auto &backtrace) { + @autoreleasepool { + // in test, we'll overwrite the sample's timestamp to one mocked by SentryCurrentDate + // etal. Doing this in a unified way between tests and production required extensive + // changes to the C++ layer, so we opted for this solution to avoid any potential + // breakages or performance hits there. +# if defined(TEST) || defined(TESTCI) + Backtrace backtraceCopy = backtrace; + backtraceCopy.absoluteTimestamp = SentryCurrentDate.systemTime; + [state appendBacktrace:backtraceCopy]; +# else + [state appendBacktrace:backtrace]; +# endif // defined(TEST) || defined(TESTCI) + } + }, + kSentryProfilerFrequencyHz); _profiler->startSampling(); [self startMetricProfiler]; diff --git a/Sources/Sentry/include/SentryProfiler+Private.h b/Sources/Sentry/include/SentryProfiler+Private.h index 9b39e7d899f..8ba1a27e6f5 100644 --- a/Sources/Sentry/include/SentryProfiler+Private.h +++ b/Sources/Sentry/include/SentryProfiler+Private.h @@ -1,6 +1,6 @@ -#import #import "SentryBacktrace.hpp" #import "SentryProfilingConditionals.h" +#import #if SENTRY_TARGET_PROFILING_SUPPORTED @@ -12,8 +12,10 @@ NS_ASSUME_NONNULL_BEGIN @property (nonatomic, strong, readonly) NSMutableArray *samples; @property (nonatomic, strong, readonly) NSMutableArray *> *stacks; @property (nonatomic, strong, readonly) NSMutableArray *> *frames; -@property (nonatomic, strong, readonly) NSMutableDictionary *threadMetadata; -@property (nonatomic, strong, readonly) NSMutableDictionary *queueMetadata; +@property (nonatomic, strong, readonly) + NSMutableDictionary *threadMetadata; +@property (nonatomic, strong, readonly) + NSMutableDictionary *queueMetadata; /* * Maintain an index of unique frames to avoid duplicating large amounts of data. Every @@ -49,8 +51,10 @@ NS_ASSUME_NONNULL_BEGIN * { stack_id: 1, ... } * ] */ -@property (nonatomic, strong, readonly) NSMutableDictionary *frameIndexLookup; -@property (nonatomic, strong, readonly) NSMutableDictionary *stackIndexLookup; +@property (nonatomic, strong, readonly) + NSMutableDictionary *frameIndexLookup; +@property (nonatomic, strong, readonly) + NSMutableDictionary *stackIndexLookup; @end @interface SentryProfilingState : NSObject diff --git a/Sources/Sentry/include/SentryProfiler+Test.h b/Sources/Sentry/include/SentryProfiler+Test.h index 580ad78a79d..ee07199123e 100644 --- a/Sources/Sentry/include/SentryProfiler+Test.h +++ b/Sources/Sentry/include/SentryProfiler+Test.h @@ -1,6 +1,6 @@ #include "SentryBacktrace.hpp" -#import "SentryProfiler.h" #import "SentryProfiler+Private.h" +#import "SentryProfiler.h" #import "SentryProfilingConditionals.h" #if SENTRY_TARGET_PROFILING_SUPPORTED diff --git a/Tests/SentryProfilerTests/SentryProfilerTests.mm b/Tests/SentryProfilerTests/SentryProfilerTests.mm index 0ae529d5f75..2ab454a1e4c 100644 --- a/Tests/SentryProfilerTests/SentryProfilerTests.mm +++ b/Tests/SentryProfilerTests/SentryProfilerTests.mm @@ -145,9 +145,8 @@ - (void)testProfilerMutationDuringSlicing mutateExpectation.expectedFulfillmentCount = operations; void (^mutateBlock)(void) = ^(void) { - [state mutate:^(__unused SentryProfilingMutableState *mutableState) { - [mutateExpectation fulfill]; - }]; + [state mutate:^( + __unused SentryProfilingMutableState *mutableState) { [mutateExpectation fulfill]; }]; }; const auto sliceOperations = [[NSOperationQueue alloc] init]; // concurrent queue @@ -195,7 +194,7 @@ - (void)testProfilerMutationDuringSerialization backtrace.absoluteTimestamp = 1; [state appendBacktrace:backtrace]; - + backtrace.absoluteTimestamp = 2; [state appendBacktrace:backtrace]; } @@ -335,18 +334,19 @@ - (void)testProfilerPayload // record a third backtrace that's identical to the second to test stack/frame deduplication [state appendBacktrace:backtrace2]; - + [state mutate:^(SentryProfilingMutableState *mutableState) { XCTAssertEqual(mutableState.frames.count, 5UL); const auto expectedOrdered = @[ - @"0x0000000000000123", @"0x0000000000000456", @"0x0000000000000789", @"0x0000000000000777", - @"0x0000000000000888" + @"0x0000000000000123", @"0x0000000000000456", @"0x0000000000000789", + @"0x0000000000000777", @"0x0000000000000888" ]; - [mutableState.frames enumerateObjectsUsingBlock:^(NSDictionary *_Nonnull actualFrame, - NSUInteger idx, __unused BOOL *_Nonnull stop) { - XCTAssert([actualFrame[@"instruction_addr"] isEqualToString:expectedOrdered[idx]]); - }]; - + [mutableState.frames + enumerateObjectsUsingBlock:^(NSDictionary *_Nonnull actualFrame, + NSUInteger idx, __unused BOOL *_Nonnull stop) { + XCTAssert([actualFrame[@"instruction_addr"] isEqualToString:expectedOrdered[idx]]); + }]; + XCTAssertEqual(mutableState.stacks.count, 2UL); XCTAssertEqual(mutableState.samples.count, 3UL); }]; From 69d8f8abffbbe94519758eb00fcea1169a4fbb99 Mon Sep 17 00:00:00 2001 From: Indragie Karunaratne Date: Mon, 15 May 2023 23:12:57 -0700 Subject: [PATCH 4/7] Remove @autoreleasepool --- Sources/Sentry/SentryProfiler.mm | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/Sources/Sentry/SentryProfiler.mm b/Sources/Sentry/SentryProfiler.mm index 7bc033dd24a..73f3eb80ac5 100644 --- a/Sources/Sentry/SentryProfiler.mm +++ b/Sources/Sentry/SentryProfiler.mm @@ -697,19 +697,17 @@ - (void)start _state = state; _profiler = std::make_shared( [state](auto &backtrace) { - @autoreleasepool { - // in test, we'll overwrite the sample's timestamp to one mocked by SentryCurrentDate - // etal. Doing this in a unified way between tests and production required extensive - // changes to the C++ layer, so we opted for this solution to avoid any potential - // breakages or performance hits there. + // in test, we'll overwrite the sample's timestamp to one mocked by SentryCurrentDate + // etal. Doing this in a unified way between tests and production required extensive + // changes to the C++ layer, so we opted for this solution to avoid any potential + // breakages or performance hits there. # if defined(TEST) || defined(TESTCI) - Backtrace backtraceCopy = backtrace; - backtraceCopy.absoluteTimestamp = SentryCurrentDate.systemTime; - [state appendBacktrace:backtraceCopy]; + Backtrace backtraceCopy = backtrace; + backtraceCopy.absoluteTimestamp = SentryCurrentDate.systemTime; + [state appendBacktrace:backtraceCopy]; # else - [state appendBacktrace:backtrace]; + [state appendBacktrace:backtrace]; # endif // defined(TEST) || defined(TESTCI) - } }, kSentryProfilerFrequencyHz); _profiler->startSampling(); From 9479742faa4a9e7514b79e3d37ef2f87b8f72063 Mon Sep 17 00:00:00 2001 From: Indragie Karunaratne Date: Mon, 15 May 2023 23:13:54 -0700 Subject: [PATCH 5/7] Update CHANGELOG.md --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 35771ff681e..5ea73ab4a74 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,7 +4,7 @@ ### Fixed -- Fix crashes in profiling serialization race condition (#3018) +- Fix crashes in profiling serialization race condition (#3018, #3035) ## 8.7.1 From 81abade8a4338aad8e6d97825c4f2d9c2e93a6d5 Mon Sep 17 00:00:00 2001 From: Sentry Github Bot Date: Tue, 16 May 2023 06:14:10 +0000 Subject: [PATCH 6/7] Format code --- Sources/Sentry/SentryProfiler.mm | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Sources/Sentry/SentryProfiler.mm b/Sources/Sentry/SentryProfiler.mm index 73f3eb80ac5..09923cc5c37 100644 --- a/Sources/Sentry/SentryProfiler.mm +++ b/Sources/Sentry/SentryProfiler.mm @@ -697,10 +697,10 @@ - (void)start _state = state; _profiler = std::make_shared( [state](auto &backtrace) { - // in test, we'll overwrite the sample's timestamp to one mocked by SentryCurrentDate - // etal. Doing this in a unified way between tests and production required extensive - // changes to the C++ layer, so we opted for this solution to avoid any potential - // breakages or performance hits there. + // in test, we'll overwrite the sample's timestamp to one mocked by SentryCurrentDate + // etal. Doing this in a unified way between tests and production required extensive + // changes to the C++ layer, so we opted for this solution to avoid any potential + // breakages or performance hits there. # if defined(TEST) || defined(TESTCI) Backtrace backtraceCopy = backtrace; backtraceCopy.absoluteTimestamp = SentryCurrentDate.systemTime; From 1ce7da96748546bf19b7a1af901d6628dbe87bf8 Mon Sep 17 00:00:00 2001 From: Indragie Karunaratne Date: Tue, 16 May 2023 08:18:39 -0700 Subject: [PATCH 7/7] Update CHANGELOG.md --- CHANGELOG.md | 1 - 1 file changed, 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index cb0c9ba26e1..6b483c078c6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,7 +5,6 @@ ### Fixed - Fix crashes in profiling serialization race condition (#3018, #3035) -- Fix crashes in profiling serialization race condition (#3018) - Fix a crash for user interaction transactions (#3036) ## 8.7.1