From 7fe7ee1da315877117436943d7e6dd88619adfde Mon Sep 17 00:00:00 2001 From: Andrew McKnight Date: Wed, 5 Apr 2023 20:33:50 -0800 Subject: [PATCH 1/8] fix: use new API to fix test compilation error --- .../UIViewController/SentryTimeToDisplayTrackerTest.swift | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/Tests/SentryTests/Integrations/Performance/UIViewController/SentryTimeToDisplayTrackerTest.swift b/Tests/SentryTests/Integrations/Performance/UIViewController/SentryTimeToDisplayTrackerTest.swift index 4c1d3673975..4999a5f0a38 100644 --- a/Tests/SentryTests/Integrations/Performance/UIViewController/SentryTimeToDisplayTrackerTest.swift +++ b/Tests/SentryTests/Integrations/Performance/UIViewController/SentryTimeToDisplayTrackerTest.swift @@ -238,7 +238,9 @@ class SentryTimeToDisplayTrackerTest: XCTestCase { fixture.dateProvider.setDate(date: Date(timeIntervalSince1970: 9)) let hub = TestHub(client: SentryClient(options: Options()), andScope: nil) - let tracer = SentryTracer(transactionContext: TransactionContext(operation: "Test Operation"), hub: hub, waitForChildren: true) + let tracer = SentryTracer(transactionContext: TransactionContext(operation: "Test Operation"), hub: hub, configuration: SentryTracerConfiguration(block: { config in + config.waitForChildren = true + })) let sut = fixture.getSut(for: UIViewController(), waitForFullDisplay: true) sut.start(for: tracer) From 6a5de75af71e68ccd1c61ab75e211a24b58b399a Mon Sep 17 00:00:00 2001 From: Andrew McKnight Date: Tue, 4 Apr 2023 12:50:31 -0800 Subject: [PATCH 2/8] use a version of formatHexAddress that doesnt require boxing into an NSNumber --- Sources/Sentry/SentryCrashStackEntryMapper.m | 9 +++------ Sources/Sentry/SentryDebugImageProvider.m | 6 ++---- Sources/Sentry/SentryMetricKitIntegration.m | 10 +++++----- Sources/Sentry/SentryProfiler.mm | 4 ++-- Sources/Sentry/include/SentryHexAddressFormatter.h | 6 ++++++ 5 files changed, 18 insertions(+), 17 deletions(-) diff --git a/Sources/Sentry/SentryCrashStackEntryMapper.m b/Sources/Sentry/SentryCrashStackEntryMapper.m index 4937146fc83..9c1de9db1fe 100644 --- a/Sources/Sentry/SentryCrashStackEntryMapper.m +++ b/Sources/Sentry/SentryCrashStackEntryMapper.m @@ -27,14 +27,11 @@ - (SentryFrame *)sentryCrashStackEntryToSentryFrame:(SentryCrashStackEntry)stack { SentryFrame *frame = [[SentryFrame alloc] init]; - NSNumber *symbolAddress = @(stackEntry.symbolAddress); - frame.symbolAddress = sentry_formatHexAddress(symbolAddress); + frame.symbolAddress = sentry_formatHexAddressPointer(stackEntry.symbolAddress); - NSNumber *instructionAddress = @(stackEntry.address); - frame.instructionAddress = sentry_formatHexAddress(instructionAddress); + frame.instructionAddress = sentry_formatHexAddressPointer(stackEntry.address); - NSNumber *imageAddress = @(stackEntry.imageAddress); - frame.imageAddress = sentry_formatHexAddress(imageAddress); + frame.imageAddress = sentry_formatHexAddressPointer(stackEntry.imageAddress); if (stackEntry.symbolName != NULL) { frame.function = [NSString stringWithCString:stackEntry.symbolName diff --git a/Sources/Sentry/SentryDebugImageProvider.m b/Sources/Sentry/SentryDebugImageProvider.m index fe6c82320e4..74abd4ef7e4 100644 --- a/Sources/Sentry/SentryDebugImageProvider.m +++ b/Sources/Sentry/SentryDebugImageProvider.m @@ -102,12 +102,10 @@ - (SentryDebugMeta *)fillDebugMetaFrom:(SentryCrashBinaryImage)image debugMeta.type = SentryDebugImageType; if (image.vmAddress > 0) { - NSNumber *imageVmAddress = [NSNumber numberWithUnsignedLongLong:image.vmAddress]; - debugMeta.imageVmAddress = sentry_formatHexAddress(imageVmAddress); + debugMeta.imageVmAddress = sentry_formatHexAddressPointer(image.vmAddress); } - NSNumber *imageAddress = [NSNumber numberWithUnsignedLongLong:image.address]; - debugMeta.imageAddress = sentry_formatHexAddress(imageAddress); + debugMeta.imageAddress = sentry_formatHexAddressPointer(image.address); debugMeta.imageSize = @(image.size); diff --git a/Sources/Sentry/SentryMetricKitIntegration.m b/Sources/Sentry/SentryMetricKitIntegration.m index 874aaef3b20..5362fd26861 100644 --- a/Sources/Sentry/SentryMetricKitIntegration.m +++ b/Sources/Sentry/SentryMetricKitIntegration.m @@ -386,9 +386,9 @@ - (SentryStacktrace *)convertMXFramesToSentryStacktrace:(NSEnumerator *metadata = threadMetadata[threadID]; if (metadata == nil) { @@ -118,7 +118,7 @@ for (std::vector::size_type backtraceAddressIdx = 0; backtraceAddressIdx < backtrace.addresses.size(); backtraceAddressIdx++) { const auto instructionAddress - = sentry_formatHexAddress(@(backtrace.addresses[backtraceAddressIdx])); + = sentry_formatHexAddressPointer(backtrace.addresses[backtraceAddressIdx]); const auto frameIndex = frameIndexLookup[instructionAddress]; if (frameIndex == nil) { diff --git a/Sources/Sentry/include/SentryHexAddressFormatter.h b/Sources/Sentry/include/SentryHexAddressFormatter.h index 37165aaa1ee..27236d60c32 100644 --- a/Sources/Sentry/include/SentryHexAddressFormatter.h +++ b/Sources/Sentry/include/SentryHexAddressFormatter.h @@ -5,3 +5,9 @@ sentry_formatHexAddress(NSNumber *value) { return [NSString stringWithFormat:@"0x%016llx", [value unsignedLongLongValue]]; } + +static inline NSString * +sentry_formatHexAddressPointer(uint64_t value) +{ + return [NSString stringWithFormat:@"0x%016llx", value]; +} From 4c20b80ba0e18fae1484ef59065a102a3de966c6 Mon Sep 17 00:00:00 2001 From: Andrew McKnight Date: Tue, 4 Apr 2023 15:13:55 -0800 Subject: [PATCH 3/8] replace some other instances of NSNumber-box-stringValue and stringWithFormat with sentry_format --- Sentry.xcodeproj/project.pbxproj | 8 ++--- .../SentryCrashDefaultMachineContextWrapper.m | 2 +- Sources/Sentry/SentryCrashReportConverter.m | 2 +- Sources/Sentry/SentryCrashStackEntryMapper.m | 8 ++--- Sources/Sentry/SentryDebugImageProvider.m | 6 ++-- Sources/Sentry/SentryMetricKitIntegration.m | 8 ++--- Sources/Sentry/SentryMetricProfiler.mm | 5 +-- Sources/Sentry/SentryProfiler.mm | 22 +++++------- Sources/Sentry/include/SentryFormatter.h | 36 +++++++++++++++++++ .../include/SentryHexAddressFormatter.h | 13 ------- 10 files changed, 64 insertions(+), 46 deletions(-) create mode 100644 Sources/Sentry/include/SentryFormatter.h delete mode 100644 Sources/Sentry/include/SentryHexAddressFormatter.h diff --git a/Sentry.xcodeproj/project.pbxproj b/Sentry.xcodeproj/project.pbxproj index 10409b81ad1..0c19d0e1de0 100644 --- a/Sentry.xcodeproj/project.pbxproj +++ b/Sentry.xcodeproj/project.pbxproj @@ -437,7 +437,7 @@ 7BA61CB9247BC57B00C130A8 /* SentryCrashDefaultBinaryImageProvider.h in Headers */ = {isa = PBXBuildFile; fileRef = 7BA61CB8247BC57B00C130A8 /* SentryCrashDefaultBinaryImageProvider.h */; }; 7BA61CBB247BC5D800C130A8 /* SentryCrashDefaultBinaryImageProvider.m in Sources */ = {isa = PBXBuildFile; fileRef = 7BA61CBA247BC5D800C130A8 /* SentryCrashDefaultBinaryImageProvider.m */; }; 7BA61CBD247BC6B900C130A8 /* TestSentryCrashBinaryImageProvider.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7BA61CBC247BC6B900C130A8 /* TestSentryCrashBinaryImageProvider.swift */; }; - 7BA61CBF247CEA8100C130A8 /* SentryHexAddressFormatter.h in Headers */ = {isa = PBXBuildFile; fileRef = 7BA61CBE247CEA8100C130A8 /* SentryHexAddressFormatter.h */; }; + 7BA61CBF247CEA8100C130A8 /* SentryFormatter.h in Headers */ = {isa = PBXBuildFile; fileRef = 7BA61CBE247CEA8100C130A8 /* SentryFormatter.h */; }; 7BA61CC6247CFC5F00C130A8 /* SentryCrashDefaultBinaryImageProviderTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7BA61CC5247CFC5F00C130A8 /* SentryCrashDefaultBinaryImageProviderTests.swift */; }; 7BA61CC8247D125400C130A8 /* SentryThreadInspector.h in Headers */ = {isa = PBXBuildFile; fileRef = 7BA61CC7247D125400C130A8 /* SentryThreadInspector.h */; }; 7BA61CCA247D128B00C130A8 /* SentryThreadInspector.m in Sources */ = {isa = PBXBuildFile; fileRef = 7BA61CC9247D128B00C130A8 /* SentryThreadInspector.m */; }; @@ -1313,7 +1313,7 @@ 7BA61CB8247BC57B00C130A8 /* SentryCrashDefaultBinaryImageProvider.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; name = SentryCrashDefaultBinaryImageProvider.h; path = include/SentryCrashDefaultBinaryImageProvider.h; sourceTree = ""; }; 7BA61CBA247BC5D800C130A8 /* SentryCrashDefaultBinaryImageProvider.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = SentryCrashDefaultBinaryImageProvider.m; sourceTree = ""; }; 7BA61CBC247BC6B900C130A8 /* TestSentryCrashBinaryImageProvider.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = TestSentryCrashBinaryImageProvider.swift; sourceTree = ""; }; - 7BA61CBE247CEA8100C130A8 /* SentryHexAddressFormatter.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; name = SentryHexAddressFormatter.h; path = include/SentryHexAddressFormatter.h; sourceTree = ""; }; + 7BA61CBE247CEA8100C130A8 /* SentryFormatter.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; name = SentryFormatter.h; path = include/SentryFormatter.h; sourceTree = ""; }; 7BA61CC5247CFC5F00C130A8 /* SentryCrashDefaultBinaryImageProviderTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SentryCrashDefaultBinaryImageProviderTests.swift; sourceTree = ""; }; 7BA61CC7247D125400C130A8 /* SentryThreadInspector.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; name = SentryThreadInspector.h; path = include/SentryThreadInspector.h; sourceTree = ""; }; 7BA61CC9247D128B00C130A8 /* SentryThreadInspector.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = SentryThreadInspector.m; sourceTree = ""; }; @@ -1993,7 +1993,7 @@ 7BA61CBA247BC5D800C130A8 /* SentryCrashDefaultBinaryImageProvider.m */, 7BA61CAA247BA98100C130A8 /* SentryDebugImageProvider.h */, 7BA61CAC247BAA0B00C130A8 /* SentryDebugImageProvider.m */, - 7BA61CBE247CEA8100C130A8 /* SentryHexAddressFormatter.h */, + 7BA61CBE247CEA8100C130A8 /* SentryFormatter.h */, 7BA61CC7247D125400C130A8 /* SentryThreadInspector.h */, 7BA61CC9247D128B00C130A8 /* SentryThreadInspector.m */, 7B7D872B2486480B00D2ECFF /* SentryStacktraceBuilder.h */, @@ -3465,7 +3465,7 @@ 03F84D2127DD414C008FE43F /* SentrySamplingProfiler.hpp in Headers */, 63FE712B20DA4C1100CDBAE8 /* SentryCrashStackCursor.h in Headers */, D8C67E9C28000E24007E326E /* SentryScreenshot.h in Headers */, - 7BA61CBF247CEA8100C130A8 /* SentryHexAddressFormatter.h in Headers */, + 7BA61CBF247CEA8100C130A8 /* SentryFormatter.h in Headers */, 7B8713B226415B7A006D6004 /* SentryAppStartTracker.h in Headers */, D8370B6C273DF20F00F66E2D /* SentryNSURLSessionTaskSearch.h in Headers */, 7B96572026830C9100C66E25 /* SentryScopeSyncC.h in Headers */, diff --git a/Sources/Sentry/SentryCrashDefaultMachineContextWrapper.m b/Sources/Sentry/SentryCrashDefaultMachineContextWrapper.m index 3cb12a98d6d..b077d673eb0 100644 --- a/Sources/Sentry/SentryCrashDefaultMachineContextWrapper.m +++ b/Sources/Sentry/SentryCrashDefaultMachineContextWrapper.m @@ -5,8 +5,8 @@ #import "SentryCrashStackCursor.h" #import "SentryCrashStackCursor_SelfThread.h" #import "SentryCrashThread.h" +#import "SentryFormatter.h" #import "SentryFrame.h" -#import "SentryHexAddressFormatter.h" #import "SentryStacktrace.h" #import "SentryStacktraceBuilder.h" #import "SentryThread.h" diff --git a/Sources/Sentry/SentryCrashReportConverter.m b/Sources/Sentry/SentryCrashReportConverter.m index 41f2cc49e61..3824c9bd20d 100644 --- a/Sources/Sentry/SentryCrashReportConverter.m +++ b/Sources/Sentry/SentryCrashReportConverter.m @@ -5,8 +5,8 @@ #import "SentryDebugMeta.h" #import "SentryEvent.h" #import "SentryException.h" +#import "SentryFormatter.h" #import "SentryFrame.h" -#import "SentryHexAddressFormatter.h" #import "SentryInAppLogic.h" #import "SentryInternalDefines.h" #import "SentryLog.h" diff --git a/Sources/Sentry/SentryCrashStackEntryMapper.m b/Sources/Sentry/SentryCrashStackEntryMapper.m index 9c1de9db1fe..6d6f175ad81 100644 --- a/Sources/Sentry/SentryCrashStackEntryMapper.m +++ b/Sources/Sentry/SentryCrashStackEntryMapper.m @@ -1,6 +1,6 @@ #import "SentryCrashStackEntryMapper.h" +#import "SentryFormatter.h" #import "SentryFrame.h" -#import "SentryHexAddressFormatter.h" #import "SentryInAppLogic.h" #import @@ -27,11 +27,11 @@ - (SentryFrame *)sentryCrashStackEntryToSentryFrame:(SentryCrashStackEntry)stack { SentryFrame *frame = [[SentryFrame alloc] init]; - frame.symbolAddress = sentry_formatHexAddressPointer(stackEntry.symbolAddress); + frame.symbolAddress = sentry_formatHexAddressUInt64(stackEntry.symbolAddress); - frame.instructionAddress = sentry_formatHexAddressPointer(stackEntry.address); + frame.instructionAddress = sentry_formatHexAddressUInt64(stackEntry.address); - frame.imageAddress = sentry_formatHexAddressPointer(stackEntry.imageAddress); + frame.imageAddress = sentry_formatHexAddressUInt64(stackEntry.imageAddress); if (stackEntry.symbolName != NULL) { frame.function = [NSString stringWithCString:stackEntry.symbolName diff --git a/Sources/Sentry/SentryDebugImageProvider.m b/Sources/Sentry/SentryDebugImageProvider.m index 74abd4ef7e4..ee93fbfa471 100644 --- a/Sources/Sentry/SentryDebugImageProvider.m +++ b/Sources/Sentry/SentryDebugImageProvider.m @@ -3,8 +3,8 @@ #import "SentryCrashDynamicLinker.h" #import "SentryCrashUUIDConversion.h" #import "SentryDebugMeta.h" +#import "SentryFormatter.h" #import "SentryFrame.h" -#import "SentryHexAddressFormatter.h" #import "SentryInternalDefines.h" #import "SentryLog.h" #import "SentryStacktrace.h" @@ -102,10 +102,10 @@ - (SentryDebugMeta *)fillDebugMetaFrom:(SentryCrashBinaryImage)image debugMeta.type = SentryDebugImageType; if (image.vmAddress > 0) { - debugMeta.imageVmAddress = sentry_formatHexAddressPointer(image.vmAddress); + debugMeta.imageVmAddress = sentry_formatHexAddressUInt64(image.vmAddress); } - debugMeta.imageAddress = sentry_formatHexAddressPointer(image.address); + debugMeta.imageAddress = sentry_formatHexAddressUInt64(image.address); debugMeta.imageSize = @(image.size); diff --git a/Sources/Sentry/SentryMetricKitIntegration.m b/Sources/Sentry/SentryMetricKitIntegration.m index 5362fd26861..6ce98f6b174 100644 --- a/Sources/Sentry/SentryMetricKitIntegration.m +++ b/Sources/Sentry/SentryMetricKitIntegration.m @@ -5,8 +5,8 @@ #import #import #import +#import #import -#import #import #import #import @@ -386,9 +386,9 @@ - (SentryStacktrace *)convertMXFramesToSentryStacktrace:(NSEnumerator *frameIndexLookup, NSMutableDictionary *stackIndexLookup) { - const auto threadID = [@(backtrace.threadMetadata.threadID) stringValue]; + const auto threadID = SENTRY_UINT64_TO_STRING(backtrace.threadMetadata.threadID); NSString *queueAddress = nil; if (backtrace.queueMetadata.address != 0) { - queueAddress = sentry_formatHexAddressPointer(backtrace.queueMetadata.address); + queueAddress = sentry_formatHexAddressUInt64(backtrace.queueMetadata.address); } NSMutableDictionary *metadata = threadMetadata[threadID]; if (metadata == nil) { @@ -118,7 +118,7 @@ for (std::vector::size_type backtraceAddressIdx = 0; backtraceAddressIdx < backtrace.addresses.size(); backtraceAddressIdx++) { const auto instructionAddress - = sentry_formatHexAddressPointer(backtrace.addresses[backtraceAddressIdx]); + = sentry_formatHexAddressUInt64(backtrace.addresses[backtraceAddressIdx]); const auto frameIndex = frameIndexLookup[instructionAddress]; if (frameIndex == nil) { @@ -180,12 +180,6 @@ } } -NSString * -serializedUnsigned64BitInteger(uint64_t value) -{ - return [NSString stringWithFormat:@"%llu", value]; -} - # if SENTRY_HAS_UIKIT /** * Convert the data structure that records timestamps for GPU frame render info from @@ -225,7 +219,7 @@ = getDurationNs(relativeFrameRenderStart, relativeFrameRenderEnd); [relativeFrameInfo addObject:@{ - @"elapsed_since_start_ns" : serializedUnsigned64BitInteger(relativeFrameRenderStart), + @"elapsed_since_start_ns" : SENTRY_UINT64_TO_STRING(relativeFrameRenderStart), @"value" : @(frameRenderDurationNs), }]; }]; @@ -255,7 +249,7 @@ const auto relativeTimestamp = getDurationNs(transaction.startSystemTime, timestamp); [relativeFrameRates addObject:@ { - @"elapsed_since_start_ns" : serializedUnsigned64BitInteger(relativeTimestamp), + @"elapsed_since_start_ns" : SENTRY_UINT64_TO_STRING(relativeTimestamp), @"value" : refreshRate, }]; }]; @@ -279,9 +273,9 @@ return; } const auto dict = [NSMutableDictionary dictionaryWithDictionary:@ { - @"elapsed_since_start_ns" : serializedUnsigned64BitInteger( + @"elapsed_since_start_ns" : SENTRY_UINT64_TO_STRING( getDurationNs(transaction.startSystemTime, sample.absoluteTimestamp)), - @"thread_id" : serializedUnsigned64BitInteger(sample.threadID), + @"thread_id" : SENTRY_UINT64_TO_STRING(sample.threadID), @"stack_id" : sample.stackIndex, }]; if (sample.queueAddress) { diff --git a/Sources/Sentry/include/SentryFormatter.h b/Sources/Sentry/include/SentryFormatter.h new file mode 100644 index 00000000000..5fcd5d59a23 --- /dev/null +++ b/Sources/Sentry/include/SentryFormatter.h @@ -0,0 +1,36 @@ +#import + +/** + * Given a format string and arguments to format, return an NSString encapsulating the result. + * Prefer use of this over @c +[NSString @c stringWithFormat:] or @c -[@(someInt) @c stringValue] . + * @note Only meant for strings where length isn't known at compile time. + */ +static inline NSString * +sentry_format(const char *format, ...) +{ + va_list args; + va_start(args, format); + int bufferSize = snprintf(NULL, 0, format, args) + 1; + char *buffer = (char *)malloc(bufferSize); + snprintf(buffer, bufferSize, format, args); + va_end(args); + + NSString *nsString = [NSString stringWithUTF8String:buffer]; + free(buffer); + + return nsString; +} + +#define SENTRY_UINT64_TO_STRING(variable) sentry_format("%llu", variable) + +static inline NSString * +sentry_formatHexAddress(NSNumber *value) +{ + return sentry_format("0x%016llx", [value unsignedLongLongValue]); +} + +static inline NSString * +sentry_formatHexAddressUInt64(uint64_t value) +{ + return sentry_format("0x%016llx", value); +} diff --git a/Sources/Sentry/include/SentryHexAddressFormatter.h b/Sources/Sentry/include/SentryHexAddressFormatter.h deleted file mode 100644 index 27236d60c32..00000000000 --- a/Sources/Sentry/include/SentryHexAddressFormatter.h +++ /dev/null @@ -1,13 +0,0 @@ -#import - -static inline NSString * -sentry_formatHexAddress(NSNumber *value) -{ - return [NSString stringWithFormat:@"0x%016llx", [value unsignedLongLongValue]]; -} - -static inline NSString * -sentry_formatHexAddressPointer(uint64_t value) -{ - return [NSString stringWithFormat:@"0x%016llx", value]; -} From c269ef0cfcc4ca6164b1f5378d4a29883b3f46b6 Mon Sep 17 00:00:00 2001 From: Andrew McKnight Date: Tue, 4 Apr 2023 23:43:37 -0800 Subject: [PATCH 4/8] test: add test cases for formatter functions --- Sentry.xcodeproj/project.pbxproj | 4 +++ Tests/SentryTests/SentryFormatterTests.swift | 32 +++++++++++++++++++ .../SentryTests/SentryTests-Bridging-Header.h | 1 + 3 files changed, 37 insertions(+) create mode 100644 Tests/SentryTests/SentryFormatterTests.swift diff --git a/Sentry.xcodeproj/project.pbxproj b/Sentry.xcodeproj/project.pbxproj index 0c19d0e1de0..b36f4c509bf 100644 --- a/Sentry.xcodeproj/project.pbxproj +++ b/Sentry.xcodeproj/project.pbxproj @@ -627,6 +627,7 @@ 8453421228BE855D00C22EEC /* SentrySampleDecision.m in Sources */ = {isa = PBXBuildFile; fileRef = 8453421128BE855D00C22EEC /* SentrySampleDecision.m */; }; 8453421628BE8A9500C22EEC /* SentrySpanStatus.m in Sources */ = {isa = PBXBuildFile; fileRef = 8453421528BE8A9500C22EEC /* SentrySpanStatus.m */; }; 8454CF8D293EAF9A006AC140 /* SentryMetricProfiler.mm in Sources */ = {isa = PBXBuildFile; fileRef = 8454CF8B293EAF9A006AC140 /* SentryMetricProfiler.mm */; }; + 84A5D7A229DD5B4300388BFA /* SentryFormatterTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 84A5D7A129DD5B4300388BFA /* SentryFormatterTests.swift */; }; 84A888FD28D9B11700C51DFD /* SentryProfiler+Test.h in Headers */ = {isa = PBXBuildFile; fileRef = 84A888FC28D9B11700C51DFD /* SentryProfiler+Test.h */; }; 84A8891C28DBD28900C51DFD /* SentryDevice.h in Headers */ = {isa = PBXBuildFile; fileRef = 84A8891A28DBD28900C51DFD /* SentryDevice.h */; }; 84A8891D28DBD28900C51DFD /* SentryDevice.mm in Sources */ = {isa = PBXBuildFile; fileRef = 84A8891B28DBD28900C51DFD /* SentryDevice.mm */; }; @@ -1537,6 +1538,7 @@ 849472802971C107002603DE /* SentrySystemWrapperTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SentrySystemWrapperTests.swift; sourceTree = ""; }; 849472822971C2CD002603DE /* SentryNSProcessInfoWrapperTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SentryNSProcessInfoWrapperTests.swift; sourceTree = ""; }; 849472842971C41A002603DE /* SentryNSTimerWrapperTest.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SentryNSTimerWrapperTest.swift; sourceTree = ""; }; + 84A5D7A129DD5B4300388BFA /* SentryFormatterTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; name = SentryFormatterTests.swift; path = "/Users/andrewmcknight/Code/organization/getsentry/repos/public/sentry-cocoa/Tests/SentryTests/SentryFormatterTests.swift"; sourceTree = ""; }; 84A888FC28D9B11700C51DFD /* SentryProfiler+Test.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = "SentryProfiler+Test.h"; path = "Sources/Sentry/include/SentryProfiler+Test.h"; sourceTree = SOURCE_ROOT; }; 84A8891A28DBD28900C51DFD /* SentryDevice.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; name = SentryDevice.h; path = include/SentryDevice.h; sourceTree = ""; }; 84A8891B28DBD28900C51DFD /* SentryDevice.mm */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.objcpp; path = SentryDevice.mm; sourceTree = ""; }; @@ -2692,6 +2694,7 @@ 7BD7299B24654CD500EA3610 /* Helper */ = { isa = PBXGroup; children = ( + 84A5D7A129DD5B4300388BFA /* SentryFormatterTests.swift */, 7B88F30324BC8E6500ADF90A /* SentrySerializationTests.swift */, 15E0A8EF240F638200F044E3 /* SentrySerializationNilTests.m */, 7BA61EA525F21E660008CAA2 /* SentryLogTests.swift */, @@ -4244,6 +4247,7 @@ 7BC6EC14255C415E0059822A /* SentryExceptionTests.swift in Sources */, 7B82722927A319E900F4BFF4 /* SentryAutoSessionTrackingIntegrationTests.swift in Sources */, D875ED0B276CC84700422FAC /* SentryNSDataTrackerTests.swift in Sources */, + 84A5D7A229DD5B4300388BFA /* SentryFormatterTests.swift in Sources */, 8ED2D28026A6581C00CA8329 /* NSURLProtocolSwizzle.m in Sources */, D808FB92281BF6EC009A2A33 /* SentryUIEventTrackingIntegrationTests.swift in Sources */, 7BC6EC04255C235F0059822A /* SentryFrameTests.swift in Sources */, diff --git a/Tests/SentryTests/SentryFormatterTests.swift b/Tests/SentryTests/SentryFormatterTests.swift new file mode 100644 index 00000000000..ee0054a7b24 --- /dev/null +++ b/Tests/SentryTests/SentryFormatterTests.swift @@ -0,0 +1,32 @@ +import XCTest + +final class SentryFormatterTests: XCTestCase { + func testFormatHexAddress() { + for (input, expected) in [ + (2_391_813_104, "0x000000008e902bf0"), + (2_412_813_376, "0x000000008fd09c40"), + (2_488_998_912, "0x00000000945b1c00") + ] { + XCTAssertEqual(sentry_formatHexAddress(input as NSNumber), expected) + } + } + + func testStringForUInt64() { + for (input, expected) in [ + (0, "0"), + (1, "1"), + (123_456, "123456"), + (UInt64.max, "18446744073709551615") + ] { + XCTAssertEqual(sentry_stringForUInt64(UInt64(input)), expected) + } + } + + func testFormatHexAddressPerformance() { + measure { + for _ in 0..<1_000 { + sentry_formatHexAddress(arc4random() as NSNumber) + } + } + } +} diff --git a/Tests/SentryTests/SentryTests-Bridging-Header.h b/Tests/SentryTests/SentryTests-Bridging-Header.h index 8e32aee1441..1dec8eee94c 100644 --- a/Tests/SentryTests/SentryTests-Bridging-Header.h +++ b/Tests/SentryTests/SentryTests-Bridging-Header.h @@ -79,6 +79,7 @@ #import "SentryFileIOTrackingIntegration.h" #import "SentryFileManager+TestProperties.h" #import "SentryFileManager.h" +#import "SentryFormatter.h" #import "SentryFrame.h" #import "SentryFrameRemover.h" #import "SentryFramesTracker+TestInit.h" From 4c589ba38373d0370aacbd64a1efe8e82da0325b Mon Sep 17 00:00:00 2001 From: Andrew McKnight Date: Tue, 4 Apr 2023 23:45:46 -0800 Subject: [PATCH 5/8] refactor implementation - don't use macro or vargs - separate into hex address and uint64 functions - use hardcoded buffer size for hex address version --- Sources/Sentry/SentryMetricProfiler.mm | 2 +- Sources/Sentry/SentryProfiler.mm | 10 +++---- Sources/Sentry/include/SentryFormatter.h | 36 ++++++++++++------------ 3 files changed, 24 insertions(+), 24 deletions(-) diff --git a/Sources/Sentry/SentryMetricProfiler.mm b/Sources/Sentry/SentryMetricProfiler.mm index 813fa18a58d..bb7f441ebb4 100644 --- a/Sources/Sentry/SentryMetricProfiler.mm +++ b/Sources/Sentry/SentryMetricProfiler.mm @@ -61,7 +61,7 @@ @implementation SentryMetricReading = getDurationNs(transaction.startSystemTime, reading.absoluteTimestamp); [timestampNormalizedValues addObject:@ { - @"elapsed_since_start_ns" : SENTRY_UINT64_TO_STRING(relativeTimestamp), + @"elapsed_since_start_ns" : sentry_stringForUInt64(relativeTimestamp), @"value" : reading.value }]; }]; diff --git a/Sources/Sentry/SentryProfiler.mm b/Sources/Sentry/SentryProfiler.mm index 51b8345a3f1..7e901bf78a1 100644 --- a/Sources/Sentry/SentryProfiler.mm +++ b/Sources/Sentry/SentryProfiler.mm @@ -86,7 +86,7 @@ NSMutableDictionary *frameIndexLookup, NSMutableDictionary *stackIndexLookup) { - const auto threadID = SENTRY_UINT64_TO_STRING(backtrace.threadMetadata.threadID); + const auto threadID = sentry_stringForUInt64(backtrace.threadMetadata.threadID); NSString *queueAddress = nil; if (backtrace.queueMetadata.address != 0) { @@ -219,7 +219,7 @@ = getDurationNs(relativeFrameRenderStart, relativeFrameRenderEnd); [relativeFrameInfo addObject:@{ - @"elapsed_since_start_ns" : SENTRY_UINT64_TO_STRING(relativeFrameRenderStart), + @"elapsed_since_start_ns" : sentry_stringForUInt64(relativeFrameRenderStart), @"value" : @(frameRenderDurationNs), }]; }]; @@ -249,7 +249,7 @@ const auto relativeTimestamp = getDurationNs(transaction.startSystemTime, timestamp); [relativeFrameRates addObject:@ { - @"elapsed_since_start_ns" : SENTRY_UINT64_TO_STRING(relativeTimestamp), + @"elapsed_since_start_ns" : sentry_stringForUInt64(relativeTimestamp), @"value" : refreshRate, }]; }]; @@ -273,9 +273,9 @@ return; } const auto dict = [NSMutableDictionary dictionaryWithDictionary:@ { - @"elapsed_since_start_ns" : SENTRY_UINT64_TO_STRING( + @"elapsed_since_start_ns" : sentry_stringForUInt64( getDurationNs(transaction.startSystemTime, sample.absoluteTimestamp)), - @"thread_id" : SENTRY_UINT64_TO_STRING(sample.threadID), + @"thread_id" : sentry_stringForUInt64(sample.threadID), @"stack_id" : sample.stackIndex, }]; if (sample.queueAddress) { diff --git a/Sources/Sentry/include/SentryFormatter.h b/Sources/Sentry/include/SentryFormatter.h index 5fcd5d59a23..4938bb1372d 100644 --- a/Sources/Sentry/include/SentryFormatter.h +++ b/Sources/Sentry/include/SentryFormatter.h @@ -1,36 +1,36 @@ #import -/** - * Given a format string and arguments to format, return an NSString encapsulating the result. - * Prefer use of this over @c +[NSString @c stringWithFormat:] or @c -[@(someInt) @c stringValue] . - * @note Only meant for strings where length isn't known at compile time. - */ +// 2 for the 0x prefix, plus 16 for the hex value, plus 1 for the null terminator +#define SENTRY_HEX_ADDRESS_LENGTH 19 + static inline NSString * -sentry_format(const char *format, ...) +sentry_snprintfHexAddress(uint64_t value) { - va_list args; - va_start(args, format); - int bufferSize = snprintf(NULL, 0, format, args) + 1; - char *buffer = (char *)malloc(bufferSize); - snprintf(buffer, bufferSize, format, args); - va_end(args); + char buffer[SENTRY_HEX_ADDRESS_LENGTH]; + snprintf(buffer, SENTRY_HEX_ADDRESS_LENGTH, "0x%016llx", value); + NSString *nsString = [NSString stringWithCString:buffer encoding:NSASCIIStringEncoding]; + return nsString; +} - NSString *nsString = [NSString stringWithUTF8String:buffer]; +static inline NSString * +sentry_stringForUInt64(uint64_t value) +{ + int bufferSize = snprintf(NULL, 0, "%llu", value) + 1; + char *buffer = (char *)malloc(bufferSize); + snprintf(buffer, bufferSize, "%llu", value); + NSString *nsString = [NSString stringWithCString:buffer encoding:NSASCIIStringEncoding]; free(buffer); - return nsString; } -#define SENTRY_UINT64_TO_STRING(variable) sentry_format("%llu", variable) - static inline NSString * sentry_formatHexAddress(NSNumber *value) { - return sentry_format("0x%016llx", [value unsignedLongLongValue]); + return sentry_snprintfHexAddress([value unsignedLongLongValue]); } static inline NSString * sentry_formatHexAddressUInt64(uint64_t value) { - return sentry_format("0x%016llx", value); + return sentry_snprintfHexAddress(value); } From 22d47ab0c22f00c891cc5c36d82a755f1b8217e2 Mon Sep 17 00:00:00 2001 From: Andrew McKnight Date: Wed, 5 Apr 2023 20:40:00 -0800 Subject: [PATCH 6/8] make tests more readable --- Tests/SentryTests/SentryFormatterTests.swift | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Tests/SentryTests/SentryFormatterTests.swift b/Tests/SentryTests/SentryFormatterTests.swift index ee0054a7b24..f3e82ec334a 100644 --- a/Tests/SentryTests/SentryFormatterTests.swift +++ b/Tests/SentryTests/SentryFormatterTests.swift @@ -3,9 +3,9 @@ import XCTest final class SentryFormatterTests: XCTestCase { func testFormatHexAddress() { for (input, expected) in [ - (2_391_813_104, "0x000000008e902bf0"), - (2_412_813_376, "0x000000008fd09c40"), - (2_488_998_912, "0x00000000945b1c00") + (0x000000008e902bf0, "0x000000008e902bf0"), + (0x000000008fd09c40, "0x000000008fd09c40"), + (0x00000000945b1c00, "0x00000000945b1c00") ] { XCTAssertEqual(sentry_formatHexAddress(input as NSNumber), expected) } From b596f4b1cb1beb624c98d143efcf84b12c8966d8 Mon Sep 17 00:00:00 2001 From: Andrew McKnight Date: Fri, 7 Apr 2023 13:26:55 -0800 Subject: [PATCH 7/8] fix path to new test file --- Sentry.xcodeproj/project.pbxproj | 8 ++++---- Tests/SentryTests/{ => Helper}/SentryFormatterTests.swift | 0 2 files changed, 4 insertions(+), 4 deletions(-) rename Tests/SentryTests/{ => Helper}/SentryFormatterTests.swift (100%) diff --git a/Sentry.xcodeproj/project.pbxproj b/Sentry.xcodeproj/project.pbxproj index b36f4c509bf..f6d29859674 100644 --- a/Sentry.xcodeproj/project.pbxproj +++ b/Sentry.xcodeproj/project.pbxproj @@ -627,7 +627,7 @@ 8453421228BE855D00C22EEC /* SentrySampleDecision.m in Sources */ = {isa = PBXBuildFile; fileRef = 8453421128BE855D00C22EEC /* SentrySampleDecision.m */; }; 8453421628BE8A9500C22EEC /* SentrySpanStatus.m in Sources */ = {isa = PBXBuildFile; fileRef = 8453421528BE8A9500C22EEC /* SentrySpanStatus.m */; }; 8454CF8D293EAF9A006AC140 /* SentryMetricProfiler.mm in Sources */ = {isa = PBXBuildFile; fileRef = 8454CF8B293EAF9A006AC140 /* SentryMetricProfiler.mm */; }; - 84A5D7A229DD5B4300388BFA /* SentryFormatterTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 84A5D7A129DD5B4300388BFA /* SentryFormatterTests.swift */; }; + 849AC40029E0C1FF00889C16 /* SentryFormatterTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 849AC3FF29E0C1FF00889C16 /* SentryFormatterTests.swift */; }; 84A888FD28D9B11700C51DFD /* SentryProfiler+Test.h in Headers */ = {isa = PBXBuildFile; fileRef = 84A888FC28D9B11700C51DFD /* SentryProfiler+Test.h */; }; 84A8891C28DBD28900C51DFD /* SentryDevice.h in Headers */ = {isa = PBXBuildFile; fileRef = 84A8891A28DBD28900C51DFD /* SentryDevice.h */; }; 84A8891D28DBD28900C51DFD /* SentryDevice.mm in Sources */ = {isa = PBXBuildFile; fileRef = 84A8891B28DBD28900C51DFD /* SentryDevice.mm */; }; @@ -1538,7 +1538,7 @@ 849472802971C107002603DE /* SentrySystemWrapperTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SentrySystemWrapperTests.swift; sourceTree = ""; }; 849472822971C2CD002603DE /* SentryNSProcessInfoWrapperTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SentryNSProcessInfoWrapperTests.swift; sourceTree = ""; }; 849472842971C41A002603DE /* SentryNSTimerWrapperTest.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SentryNSTimerWrapperTest.swift; sourceTree = ""; }; - 84A5D7A129DD5B4300388BFA /* SentryFormatterTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; name = SentryFormatterTests.swift; path = "/Users/andrewmcknight/Code/organization/getsentry/repos/public/sentry-cocoa/Tests/SentryTests/SentryFormatterTests.swift"; sourceTree = ""; }; + 849AC3FF29E0C1FF00889C16 /* SentryFormatterTests.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = SentryFormatterTests.swift; sourceTree = ""; }; 84A888FC28D9B11700C51DFD /* SentryProfiler+Test.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = "SentryProfiler+Test.h"; path = "Sources/Sentry/include/SentryProfiler+Test.h"; sourceTree = SOURCE_ROOT; }; 84A8891A28DBD28900C51DFD /* SentryDevice.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; name = SentryDevice.h; path = include/SentryDevice.h; sourceTree = ""; }; 84A8891B28DBD28900C51DFD /* SentryDevice.mm */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.objcpp; path = SentryDevice.mm; sourceTree = ""; }; @@ -2694,7 +2694,7 @@ 7BD7299B24654CD500EA3610 /* Helper */ = { isa = PBXGroup; children = ( - 84A5D7A129DD5B4300388BFA /* SentryFormatterTests.swift */, + 849AC3FF29E0C1FF00889C16 /* SentryFormatterTests.swift */, 7B88F30324BC8E6500ADF90A /* SentrySerializationTests.swift */, 15E0A8EF240F638200F044E3 /* SentrySerializationNilTests.m */, 7BA61EA525F21E660008CAA2 /* SentryLogTests.swift */, @@ -4247,12 +4247,12 @@ 7BC6EC14255C415E0059822A /* SentryExceptionTests.swift in Sources */, 7B82722927A319E900F4BFF4 /* SentryAutoSessionTrackingIntegrationTests.swift in Sources */, D875ED0B276CC84700422FAC /* SentryNSDataTrackerTests.swift in Sources */, - 84A5D7A229DD5B4300388BFA /* SentryFormatterTests.swift in Sources */, 8ED2D28026A6581C00CA8329 /* NSURLProtocolSwizzle.m in Sources */, D808FB92281BF6EC009A2A33 /* SentryUIEventTrackingIntegrationTests.swift in Sources */, 7BC6EC04255C235F0059822A /* SentryFrameTests.swift in Sources */, 0AE455AD28F584D2006680E5 /* SentryReachabilityTests.m in Sources */, 63FE720420DA66EC00CDBAE8 /* SentryCrashString_Tests.m in Sources */, + 849AC40029E0C1FF00889C16 /* SentryFormatterTests.swift in Sources */, 7BDDE3CC2966BD4700EB9177 /* SentryMXManagerTests.swift in Sources */, 7BC6EC0C255C3DF80059822A /* SentryThreadTests.swift in Sources */, D884A20527C80F6300074664 /* SentryCoreDataTrackerTest.swift in Sources */, diff --git a/Tests/SentryTests/SentryFormatterTests.swift b/Tests/SentryTests/Helper/SentryFormatterTests.swift similarity index 100% rename from Tests/SentryTests/SentryFormatterTests.swift rename to Tests/SentryTests/Helper/SentryFormatterTests.swift From 8a345884c4e1a32453fc7dd832e5e576d9c66cc1 Mon Sep 17 00:00:00 2001 From: Andrew McKnight Date: Fri, 7 Apr 2023 13:33:02 -0800 Subject: [PATCH 8/8] add comment about optimization --- Sources/Sentry/include/SentryFormatter.h | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/Sources/Sentry/include/SentryFormatter.h b/Sources/Sentry/include/SentryFormatter.h index 4938bb1372d..362a22ed465 100644 --- a/Sources/Sentry/include/SentryFormatter.h +++ b/Sources/Sentry/include/SentryFormatter.h @@ -26,6 +26,13 @@ sentry_stringForUInt64(uint64_t value) static inline NSString * sentry_formatHexAddress(NSNumber *value) { + /* + * We observed a 41% speedup by using snprintf vs +[NSString stringWithFormat:]. In a trial + * using a profile, we observed the +[NSString stringWithFormat:] using 282ms of CPU time, vs + * 164ms of CPU time for snprintf. There is also an assumed space improvement due to not needing + * to allocate as many instances of NSString, like for the format string literal, instead only + * using stack-bound C strings. + */ return sentry_snprintfHexAddress([value unsignedLongLongValue]); }