diff --git a/CHANGELOG.md b/CHANGELOG.md index 21dc5a9cc98..06830626283 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,8 +8,9 @@ ### Fixes -- Ensure the current GPU frame rate is always reported for concurrent transaction profiling metrics (#2929) +- Improved performance serializing profiling data (#2863) - Possible crash in Core Data tracking (#2865) +- Ensure the current GPU frame rate is always reported for concurrent transaction profiling metrics (#2929) ## 8.5.0 diff --git a/Sentry.xcodeproj/project.pbxproj b/Sentry.xcodeproj/project.pbxproj index a54a52950b6..9c364dc5aad 100644 --- a/Sentry.xcodeproj/project.pbxproj +++ b/Sentry.xcodeproj/project.pbxproj @@ -440,7 +440,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 */; }; @@ -630,6 +630,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 */; }; + 849AC40029E0C1FF00889C16 /* SentryFormatterTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 849AC3FF29E0C1FF00889C16 /* SentryFormatterTests.swift */; }; 84A5D75B29D5170700388BFA /* TimeInterval+Sentry.swift in Sources */ = {isa = PBXBuildFile; fileRef = 84A5D75A29D5170700388BFA /* TimeInterval+Sentry.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 */; }; @@ -1322,7 +1323,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 = ""; }; @@ -1546,6 +1547,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 = ""; }; + 849AC3FF29E0C1FF00889C16 /* SentryFormatterTests.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = SentryFormatterTests.swift; sourceTree = ""; }; 84A5D75A29D5170700388BFA /* TimeInterval+Sentry.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "TimeInterval+Sentry.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 = ""; }; @@ -2007,7 +2009,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 */, @@ -2706,6 +2708,7 @@ 7BD7299B24654CD500EA3610 /* Helper */ = { isa = PBXGroup; children = ( + 849AC3FF29E0C1FF00889C16 /* SentryFormatterTests.swift */, 7B88F30324BC8E6500ADF90A /* SentrySerializationTests.swift */, 15E0A8EF240F638200F044E3 /* SentrySerializationNilTests.m */, 7BA61EA525F21E660008CAA2 /* SentryLogTests.swift */, @@ -3484,7 +3487,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 */, @@ -4272,6 +4275,7 @@ 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/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 4937146fc83..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,14 +27,11 @@ - (SentryFrame *)sentryCrashStackEntryToSentryFrame:(SentryCrashStackEntry)stack { SentryFrame *frame = [[SentryFrame alloc] init]; - NSNumber *symbolAddress = @(stackEntry.symbolAddress); - frame.symbolAddress = sentry_formatHexAddress(symbolAddress); + frame.symbolAddress = sentry_formatHexAddressUInt64(stackEntry.symbolAddress); - NSNumber *instructionAddress = @(stackEntry.address); - frame.instructionAddress = sentry_formatHexAddress(instructionAddress); + frame.instructionAddress = sentry_formatHexAddressUInt64(stackEntry.address); - NSNumber *imageAddress = @(stackEntry.imageAddress); - frame.imageAddress = sentry_formatHexAddress(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 fe6c82320e4..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,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_formatHexAddressUInt64(image.vmAddress); } - NSNumber *imageAddress = [NSNumber numberWithUnsignedLongLong:image.address]; - debugMeta.imageAddress = sentry_formatHexAddress(imageAddress); + debugMeta.imageAddress = sentry_formatHexAddressUInt64(image.address); debugMeta.imageSize = @(image.size); diff --git a/Sources/Sentry/SentryMetricKitIntegration.m b/Sources/Sentry/SentryMetricKitIntegration.m index 874aaef3b20..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_stringForUInt64(backtrace.threadMetadata.threadID); NSString *queueAddress = nil; if (backtrace.queueMetadata.address != 0) { - queueAddress = sentry_formatHexAddress(@(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_formatHexAddress(@(backtrace.addresses[backtraceAddressIdx])); + = sentry_formatHexAddressUInt64(backtrace.addresses[backtraceAddressIdx]); const auto frameIndex = frameIndexLookup[instructionAddress]; if (frameIndex == nil) { @@ -181,12 +181,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 @@ -224,13 +218,13 @@ const auto relativeTimestamp = getDurationNs(transaction.startSystemTime, timestamp); [slicedGPUEntries addObject:@ { - @"elapsed_since_start_ns" : serializedUnsigned64BitInteger(relativeTimestamp), + @"elapsed_since_start_ns" : sentry_stringForUInt64(relativeTimestamp), @"value" : obj[@"value"], }]; }]; if (useMostRecentRecording && slicedGPUEntries.count == 0) { [slicedGPUEntries addObject:@ { - @"elapsed_since_start_ns" : serializedUnsigned64BitInteger(0), + @"elapsed_since_start_ns" : @"0", @"value" : nearestPredecessorValue, }]; } @@ -254,9 +248,9 @@ return; } const auto dict = [NSMutableDictionary dictionaryWithDictionary:@ { - @"elapsed_since_start_ns" : serializedUnsigned64BitInteger( + @"elapsed_since_start_ns" : sentry_stringForUInt64( getDurationNs(transaction.startSystemTime, sample.absoluteTimestamp)), - @"thread_id" : serializedUnsigned64BitInteger(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 new file mode 100644 index 00000000000..362a22ed465 --- /dev/null +++ b/Sources/Sentry/include/SentryFormatter.h @@ -0,0 +1,43 @@ +#import + +// 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_snprintfHexAddress(uint64_t value) +{ + char buffer[SENTRY_HEX_ADDRESS_LENGTH]; + snprintf(buffer, SENTRY_HEX_ADDRESS_LENGTH, "0x%016llx", value); + NSString *nsString = [NSString stringWithCString:buffer encoding:NSASCIIStringEncoding]; + return nsString; +} + +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; +} + +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]); +} + +static inline NSString * +sentry_formatHexAddressUInt64(uint64_t value) +{ + return sentry_snprintfHexAddress(value); +} diff --git a/Sources/Sentry/include/SentryHexAddressFormatter.h b/Sources/Sentry/include/SentryHexAddressFormatter.h deleted file mode 100644 index 37165aaa1ee..00000000000 --- a/Sources/Sentry/include/SentryHexAddressFormatter.h +++ /dev/null @@ -1,7 +0,0 @@ -#import - -static inline NSString * -sentry_formatHexAddress(NSNumber *value) -{ - return [NSString stringWithFormat:@"0x%016llx", [value unsignedLongLongValue]]; -} diff --git a/Tests/SentryTests/Helper/SentryFormatterTests.swift b/Tests/SentryTests/Helper/SentryFormatterTests.swift new file mode 100644 index 00000000000..ad181aa99e7 --- /dev/null +++ b/Tests/SentryTests/Helper/SentryFormatterTests.swift @@ -0,0 +1,24 @@ +import XCTest + +final class SentryFormatterTests: XCTestCase { + func testFormatHexAddress() { + for (input, expected) in [ + (0x000000008e902bf0, "0x000000008e902bf0"), + (0x000000008fd09c40, "0x000000008fd09c40"), + (0x00000000945b1c00, "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) + } + } +} diff --git a/Tests/SentryTests/SentryTests-Bridging-Header.h b/Tests/SentryTests/SentryTests-Bridging-Header.h index 012cbcaff01..77ec6f5ec7c 100644 --- a/Tests/SentryTests/SentryTests-Bridging-Header.h +++ b/Tests/SentryTests/SentryTests-Bridging-Header.h @@ -80,6 +80,7 @@ #import "SentryFileIOTrackingIntegration.h" #import "SentryFileManager+TestProperties.h" #import "SentryFileManager.h" +#import "SentryFormatter.h" #import "SentryFrame.h" #import "SentryFrameRemover.h" #import "SentryFramesTracker+TestInit.h"