impr: Speed up getBinaryImages V2

philipphofmann · philipphofmann · commit 27c33f0c5551 · 2024-11-15T09:44:28.000+01:00
Follow up on GH-4435. Use binary image cache to get debug meta when finishing the tracer and capturing profiles. Mark all methods to get debug meta not using the cache as deprecated to ensure we migrate to the methods using the cache.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -14,6 +14,10 @@
 - Make `Scope.span` fully thread safe (#4519)
 - Finish TTFD when not calling reportFullyDisplayed before binding a new transaction to the scope (#4526).
 
+### Improvements
+
+- impr: Speed up getBinaryImages V2 (#4539). Follow up on (#4435)
+
 ## 8.40.1
 
 ### Fixes
diff --git a/Sentry.xcodeproj/project.pbxproj b/Sentry.xcodeproj/project.pbxproj
@@ -76,6 +76,7 @@
 		620203B22C59025E0008317C /* SentryFileContents.swift in Sources */ = {isa = PBXBuildFile; fileRef = 620203B12C59025E0008317C /* SentryFileContents.swift */; };
 		620379DB2AFE1415005AC0C1 /* SentryBuildAppStartSpans.h in Headers */ = {isa = PBXBuildFile; fileRef = 620379DA2AFE1415005AC0C1 /* SentryBuildAppStartSpans.h */; };
 		620379DD2AFE1432005AC0C1 /* SentryBuildAppStartSpans.m in Sources */ = {isa = PBXBuildFile; fileRef = 620379DC2AFE1432005AC0C1 /* SentryBuildAppStartSpans.m */; };
+		6205B4A42CE73AA100744684 /* TestDebugImageProvider.swift in Sources */ = {isa = PBXBuildFile; fileRef = D85790282976A69F00C6AC1F /* TestDebugImageProvider.swift */; };
 		621AE74B2C626C230012E730 /* SentryANRTrackerV2.h in Headers */ = {isa = PBXBuildFile; fileRef = 621AE74A2C626C230012E730 /* SentryANRTrackerV2.h */; };
 		621AE74D2C626C510012E730 /* SentryANRTrackerV2.m in Sources */ = {isa = PBXBuildFile; fileRef = 621AE74C2C626C510012E730 /* SentryANRTrackerV2.m */; };
 		621C88482CAD23B9000EABCB /* SentryCaptureTransactionWithProfile.h in Headers */ = {isa = PBXBuildFile; fileRef = 621C88472CAD23B9000EABCB /* SentryCaptureTransactionWithProfile.h */; };
@@ -834,7 +835,6 @@
 		D855AD62286ED6A4002573E1 /* SentryCrashTests.m in Sources */ = {isa = PBXBuildFile; fileRef = D855AD61286ED6A4002573E1 /* SentryCrashTests.m */; };
 		D855B3E827D652AF00BCED76 /* SentryCoreDataTrackingIntegrationTest.swift in Sources */ = {isa = PBXBuildFile; fileRef = D855B3E727D652AF00BCED76 /* SentryCoreDataTrackingIntegrationTest.swift */; };
 		D855B3EA27D652C700BCED76 /* TestCoreDataStack.swift in Sources */ = {isa = PBXBuildFile; fileRef = D855B3E927D652C700BCED76 /* TestCoreDataStack.swift */; };
-		D85790292976A69F00C6AC1F /* TestDebugImageProvider.swift in Sources */ = {isa = PBXBuildFile; fileRef = D85790282976A69F00C6AC1F /* TestDebugImageProvider.swift */; };
 		D85852B627ECEEDA00C6D8AE /* SentryScreenshot.m in Sources */ = {isa = PBXBuildFile; fileRef = D85852B427ECEEDA00C6D8AE /* SentryScreenshot.m */; };
 		D85852BA27EDDC5900C6D8AE /* SentryUIApplication.m in Sources */ = {isa = PBXBuildFile; fileRef = D85852B827EDDC5900C6D8AE /* SentryUIApplication.m */; };
 		D858FA662A29EAB3002A3503 /* SentryBinaryImageCache.h in Headers */ = {isa = PBXBuildFile; fileRef = D858FA642A29EAB3002A3503 /* SentryBinaryImageCache.h */; };
@@ -3069,7 +3069,6 @@
 				7B2A70DE27D60904008B0D15 /* SentryTestThreadWrapper.swift */,
 				7B18DE4928DA0C8B004845C6 /* SentryNSNotificationCenterWrapperTests.swift */,
 				84A8892028DBD8D600C51DFD /* SentryDeviceTests.mm */,
-				D85790282976A69F00C6AC1F /* TestDebugImageProvider.swift */,
 				8431EE5A29ADB8EA00D8DC56 /* SentryTimeTests.m */,
 				62C3168A2B1F865A000D7031 /* SentryTimeSwiftTests.swift */,
 				33042A1629DC2C4300C60085 /* SentryExtraContextProviderTests.swift */,
@@ -3487,6 +3486,7 @@
 				8E25C97425F8511A00DC215B /* TestRandom.swift */,
 				7BE3C7762445E50A00A38442 /* TestCurrentDateProvider.swift */,
 				7BDB03BE25136A7D00BAE198 /* TestSentryDispatchQueueWrapper.swift */,
+				D85790282976A69F00C6AC1F /* TestDebugImageProvider.swift */,
 				7BAF3DC7243DB90E008A5414 /* TestTransport.swift */,
 				7BA0C049280563AA003E0326 /* TestTransportAdapter.swift */,
 				7B944FAF2469B46000A10721 /* TestClient.swift */,
@@ -4859,7 +4859,6 @@
 				62F05D2B2C0DB1F100916E3F /* SentryLogTestHelper.m in Sources */,
 				7B68345128F7EB3D00FB7064 /* SentryMeasurementUnitTests.swift in Sources */,
 				7B14089A248791660035403D /* SentryCrashStackEntryMapperTests.swift in Sources */,
-				D85790292976A69F00C6AC1F /* TestDebugImageProvider.swift in Sources */,
 				7B869EBC249B91D8004F4FDB /* SentryDebugMetaEquality.swift in Sources */,
 				7B01CE3D271993AC00B5AF31 /* SentryTransportFactoryTests.swift in Sources */,
 				7B30B68026527C3C006B2752 /* SentryFramesTrackerTests.swift in Sources */,
@@ -5136,6 +5135,7 @@
 				84A5D75B29D5170700388BFA /* TimeInterval+Sentry.swift in Sources */,
 				62AB8C9E2BF3925700BFC2AC /* WeakReference.swift in Sources */,
 				84AC61D929F7643B009EEF61 /* TestDispatchFactory.swift in Sources */,
+				6205B4A42CE73AA100744684 /* TestDebugImageProvider.swift in Sources */,
 				8431F01929B2852D00D8DC56 /* Invocation.swift in Sources */,
 				84B7FA4629B2935F00AD93B1 /* ClearTestState.swift in Sources */,
 				84281C622A579D0700EE88F2 /* SentryProfilerMocksSwiftCompatible.mm in Sources */,
diff --git a/SentryTestUtils/SentryTestUtils-ObjC-BridgingHeader.h b/SentryTestUtils/SentryTestUtils-ObjC-BridgingHeader.h
@@ -27,13 +27,15 @@
 #import "PrivateSentrySDKOnly.h"
 #import "SentryAppStartMeasurement.h"
 #import "SentryAppState.h"
+#import "SentryBinaryImageCache.h"
 #import "SentryClient+Private.h"
 #import "SentryClient+TestInit.h"
 #import "SentryCrash+Test.h"
 #import "SentryCrashCachedData.h"
 #import "SentryCrashInstallation+Private.h"
 #import "SentryCrashMonitor_MachException.h"
 #import "SentryCrashWrapper.h"
+#import "SentryDebugImageProvider+HybridSDKs.h"
 #import "SentryDependencyContainer.h"
 #import "SentryDispatchFactory.h"
 #import "SentryDispatchSourceWrapper.h"
diff --git a/SentryTestUtils/TestDebugImageProvider.swift b/SentryTestUtils/TestDebugImageProvider.swift
@@ -0,0 +1,36 @@
+import _SentryPrivate
+import Foundation
+@testable import Sentry
+
+public class TestDebugImageProvider: SentryDebugImageProvider {
+    public var debugImages: [DebugMeta]?
+
+    @available(*, deprecated)
+    public override func getDebugImages() -> [DebugMeta] {
+        getDebugImagesCrashed(true)
+    }
+
+    @available(*, deprecated)
+    public override func getDebugImagesCrashed(_ isCrash: Bool) -> [DebugMeta] {
+        debugImages ?? super.getDebugImagesCrashed(isCrash)
+    }
+    
+    public var getDebugImagesFromCacheForFramesInvocations = Invocations<Void>()
+    public override func getDebugImagesFromCacheForFrames(frames: [Frame]) -> [DebugMeta] {
+        getDebugImagesFromCacheForFramesInvocations.record(Void())
+        
+        return debugImages ?? super.getDebugImagesFromCacheForFrames(frames: frames)
+    }
+    
+    public var getDebugImagesFromCacheForThreadsInvocations = Invocations<Void>()
+    public override func getDebugImagesFromCacheForThreads(threads: [SentryThread]) -> [DebugMeta] {
+        getDebugImagesFromCacheForThreadsInvocations.record(Void())
+        return debugImages ?? super.getDebugImagesFromCacheForThreads(threads: threads)
+    }
+    
+    public var getDebugImagesFromCacheInvocations = Invocations<Void>()
+    public override func getDebugImagesFromCache() -> [DebugMeta] {
+        getDebugImagesFromCacheInvocations.record(Void())
+        return debugImages ?? super.getDebugImagesFromCache()
+    }
+}
diff --git a/Sources/Sentry/PrivateSentrySDKOnly.mm b/Sources/Sentry/PrivateSentrySDKOnly.mm
@@ -60,8 +60,11 @@ + (nullable SentryEnvelope *)envelopeWithData:(NSData *)data
 
 + (NSArray<SentryDebugMeta *> *)getDebugImagesCrashed:(BOOL)isCrash
 {
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wdeprecated-declarations"
     return [[SentryDependencyContainer sharedInstance].debugImageProvider
         getDebugImagesCrashed:isCrash];
+#pragma clang diagnostic pop
 }
 
 + (nullable SentryAppStartMeasurement *)appStartMeasurement
diff --git a/Sources/Sentry/Profiling/SentryProfilerSerialization.mm b/Sources/Sentry/Profiling/SentryProfilerSerialization.mm
@@ -4,6 +4,7 @@
 
 #    import "SentryClient+Private.h"
 #    import "SentryDateUtils.h"
+#    import "SentryDebugImageProvider+HybridSDKs.h"
 #    import "SentryDependencyContainer.h"
 #    import "SentryDevice.h"
 #    import "SentryEnvelope.h"
@@ -312,7 +313,7 @@
     const auto chunkID = [[SentryId alloc] init];
     const auto payload = sentry_serializedContinuousProfileChunk(
         profileID, chunkID, profileState, metricProfilerState,
-        [SentryDependencyContainer.sharedInstance.debugImageProvider getDebugImagesCrashed:NO],
+        [SentryDependencyContainer.sharedInstance.debugImageProvider getDebugImagesFromCache],
         SentrySDK.currentHub
 #    if SENTRY_HAS_UIKIT
         ,
@@ -350,12 +351,14 @@
     SentryProfiler *profiler, NSDictionary<NSString *, id> *profilingData,
     SentryTransaction *transaction, NSDate *startTimestamp)
 {
+    const auto images =
+        [SentryDependencyContainer.sharedInstance.debugImageProvider getDebugImagesFromCache];
     const auto payload = sentry_serializedTraceProfileData(
         profilingData, transaction.startSystemTime, transaction.endSystemTime,
         sentry_profilerTruncationReasonName(profiler.truncationReason),
         [profiler.metricProfiler serializeTraceProfileMetricsBetween:transaction.startSystemTime
                                                                  and:transaction.endSystemTime],
-        [SentryDependencyContainer.sharedInstance.debugImageProvider getDebugImagesCrashed:NO], hub
+        images, hub
 #    if SENTRY_HAS_UIKIT
         ,
         profiler.screenFrameData
@@ -403,7 +406,7 @@
         endSystemTime, sentry_profilerTruncationReasonName(profiler.truncationReason),
         [profiler.metricProfiler serializeTraceProfileMetricsBetween:startSystemTime
                                                                  and:endSystemTime],
-        [SentryDependencyContainer.sharedInstance.debugImageProvider getDebugImagesCrashed:NO], hub
+        [SentryDependencyContainer.sharedInstance.debugImageProvider getDebugImagesFromCache], hub
 #    if SENTRY_HAS_UIKIT
         ,
         profiler.screenFrameData
diff --git a/Sources/Sentry/Public/SentryDebugImageProvider.h b/Sources/Sentry/Public/SentryDebugImageProvider.h
@@ -31,7 +31,10 @@ NS_ASSUME_NONNULL_BEGIN
  * crash, each image's data section crash info is also included.
  */
 - (NSArray<SentryDebugMeta *> *)getDebugImagesForThreads:(NSArray<SentryThread *> *)threads
-                                                 isCrash:(BOOL)isCrash;
+                                                 isCrash:(BOOL)isCrash
+    DEPRECATED_MSG_ATTRIBUTE("This method is slow and will be removed in a future version. Use "
+                             "-[getDebugImagesFromCacheForThreads:] instead.");
+;
 
 /**
  * Returns a list of debug images that are being referenced by the given frames.
@@ -52,7 +55,9 @@ NS_ASSUME_NONNULL_BEGIN
  * crash, each image's data section crash info is also included.
  */
 - (NSArray<SentryDebugMeta *> *)getDebugImagesForFrames:(NSArray<SentryFrame *> *)frames
-                                                isCrash:(BOOL)isCrash;
+                                                isCrash:(BOOL)isCrash
+    DEPRECATED_MSG_ATTRIBUTE("This method is slow and will be removed in a future version. Use "
+                             "-[getDebugImagesFromCacheForFrames:] instead.");
 
 /**
  * Returns the current list of debug images. Be aware that the @c SentryDebugMeta is actually
@@ -71,8 +76,12 @@ NS_ASSUME_NONNULL_BEGIN
  * @param isCrash @c YES if we're collecting binary images for a crash report, @c NO if we're
  * gathering them for other backtrace information, like a performance transaction. If this is for a
  * crash, each image's data section crash info is also included.
+ *
+ * @warning This method is slow. Please consider using @c getDebugImagesFromCache.
  */
-- (NSArray<SentryDebugMeta *> *)getDebugImagesCrashed:(BOOL)isCrash;
+- (NSArray<SentryDebugMeta *> *)getDebugImagesCrashed:(BOOL)isCrash
+    DEPRECATED_MSG_ATTRIBUTE("This method is slow and will be removed in a future version. Use "
+                             "-[getDebugImagesFromCache:] instead.");
 
 @end
 
diff --git a/Sources/Sentry/SentryBinaryImageCache.m b/Sources/Sentry/SentryBinaryImageCache.m
@@ -5,6 +5,8 @@
 #import "SentryInAppLogic.h"
 #import "SentryLog.h"
 
+NS_ASSUME_NONNULL_BEGIN
+
 static void binaryImageWasAdded(const SentryCrashBinaryImage *image);
 
 static void binaryImageWasRemoved(const SentryCrashBinaryImage *image);
@@ -155,6 +157,13 @@ - (NSInteger)indexOfImage:(uint64_t)address
     return imagePaths;
 }
 
+- (NSArray<SentryBinaryImageInfo *> *)getAllBinaryImages
+{
+    @synchronized(self) {
+        return _cache.copy;
+    }
+}
+
 @end
 
 static void
@@ -168,3 +177,5 @@ - (NSInteger)indexOfImage:(uint64_t)address
 {
     [SentryDependencyContainer.sharedInstance.binaryImageCache binaryImageRemoved:image];
 }
+
+NS_ASSUME_NONNULL_END
diff --git a/Sources/Sentry/SentryDebugImageProvider.m b/Sources/Sentry/SentryDebugImageProvider.m
@@ -13,6 +13,8 @@
 #import "SentryThread.h"
 #import <Foundation/Foundation.h>
 
+NS_ASSUME_NONNULL_BEGIN
+
 @interface SentryDebugImageProvider ()
 
 @property (nonatomic, strong) id<SentryCrashBinaryImageProvider> binaryImageProvider;
@@ -50,7 +52,10 @@ - (instancetype)initWithBinaryImageProvider:(id<SentryCrashBinaryImageProvider>)
 {
     NSMutableArray<SentryDebugMeta *> *result = [NSMutableArray array];
 
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wdeprecated-declarations"
     NSArray<SentryDebugMeta *> *binaryImages = [self getDebugImagesCrashed:isCrash];
+#pragma clang diagnostic pop
 
     for (SentryDebugMeta *sourceImage in binaryImages) {
         if ([addresses containsObject:sourceImage.imageAddress]) {
@@ -134,19 +139,7 @@ - (void)extractDebugImageAddressFromFrames:(NSArray<SentryFrame *> *)frames
             continue;
         }
 
-        SentryDebugMeta *debugMeta = [[SentryDebugMeta alloc] init];
-        debugMeta.debugID = info.UUID;
-        debugMeta.type = SentryDebugImageType;
-
-        if (info.vmAddress > 0) {
-            debugMeta.imageVmAddress = sentry_formatHexAddressUInt64(info.vmAddress);
-        }
-
-        debugMeta.imageAddress = sentry_formatHexAddressUInt64(info.address);
-        debugMeta.imageSize = @(info.size);
-        debugMeta.codeFile = info.name;
-
-        [result addObject:debugMeta];
+        [result addObject:[self fillDebugMetaFromBinaryImageInfo:info]];
     }
 
     return result;
@@ -158,6 +151,17 @@ - (void)extractDebugImageAddressFromFrames:(NSArray<SentryFrame *> *)frames
     return [self getDebugImagesCrashed:YES];
 }
 
+- (NSArray<SentryDebugMeta *> *)getDebugImagesFromCache
+{
+    NSArray<SentryBinaryImageInfo *> *infos = [self.binaryImageCache getAllBinaryImages];
+    NSMutableArray<SentryDebugMeta *> *result =
+        [[NSMutableArray alloc] initWithCapacity:infos.count];
+    for (SentryBinaryImageInfo *info in infos) {
+        [result addObject:[self fillDebugMetaFromBinaryImageInfo:info]];
+    }
+    return result;
+}
+
 - (NSArray<SentryDebugMeta *> *)getDebugImagesCrashed:(BOOL)isCrash
 {
     NSMutableArray<SentryDebugMeta *> *debugMetaArray = [NSMutableArray new];
@@ -193,4 +197,23 @@ - (SentryDebugMeta *)fillDebugMetaFrom:(SentryCrashBinaryImage)image
     return debugMeta;
 }
 
+- (SentryDebugMeta *)fillDebugMetaFromBinaryImageInfo:(SentryBinaryImageInfo *)info
+{
+    SentryDebugMeta *debugMeta = [[SentryDebugMeta alloc] init];
+    debugMeta.debugID = info.UUID;
+    debugMeta.type = SentryDebugImageType;
+
+    if (info.vmAddress > 0) {
+        debugMeta.imageVmAddress = sentry_formatHexAddressUInt64(info.vmAddress);
+    }
+
+    debugMeta.imageAddress = sentry_formatHexAddressUInt64(info.address);
+    debugMeta.imageSize = @(info.size);
+    debugMeta.codeFile = info.name;
+
+    return debugMeta;
+}
+
 @end
+
+NS_ASSUME_NONNULL_END
diff --git a/Sources/Sentry/SentryTracer.m b/Sources/Sentry/SentryTracer.m
@@ -863,7 +863,7 @@ - (void)addAppStartMeasurements:(SentryTransaction *)transaction
 
             // The backend calculates statistics on the number and size of debug images for app
             // start transactions. Therefore, we add all debug images here.
-            transaction.debugMeta = [self.debugImageProvider getDebugImagesCrashed:NO];
+            transaction.debugMeta = [self.debugImageProvider getDebugImagesFromCache];
         }
     }
 }
diff --git a/Sources/Sentry/include/HybridPublic/SentryBinaryImageCache.h b/Sources/Sentry/include/HybridPublic/SentryBinaryImageCache.h
@@ -22,6 +22,8 @@ NS_ASSUME_NONNULL_BEGIN
 
 - (void)stop;
 
+- (NSArray<SentryBinaryImageInfo *> *)getAllBinaryImages;
+
 - (nullable SentryBinaryImageInfo *)imageByAddress:(const uint64_t)address;
 
 - (NSSet<NSString *> *)imagePathsForInAppInclude:(NSString *)inAppInclude;
diff --git a/Sources/Sentry/include/HybridPublic/SentryDebugImageProvider+HybridSDKs.h b/Sources/Sentry/include/HybridPublic/SentryDebugImageProvider+HybridSDKs.h
@@ -32,6 +32,9 @@ NS_ASSUME_NONNULL_BEGIN
 - (NSArray<SentryDebugMeta *> *)getDebugImagesForImageAddressesFromCache:
     (NSSet<NSString *> *)imageAddresses
     NS_SWIFT_NAME(getDebugImagesForImageAddressesFromCache(imageAddresses:));
+
+- (NSArray<SentryDebugMeta *> *)getDebugImagesFromCache;
+
 @end
 
 NS_ASSUME_NONNULL_END
diff --git a/Tests/SentryProfilerTests/SentryProfileTestFixture.swift b/Tests/SentryProfilerTests/SentryProfileTestFixture.swift
@@ -47,6 +47,19 @@ class SentryProfileTestFixture {
         SentryDependencyContainer.sharedInstance().timerFactory = timeoutTimerFactory
         SentryDependencyContainer.sharedInstance().notificationCenterWrapper = notificationCenter
         
+        let image = DebugMeta()
+        image.name = "sentrytest"
+        image.imageAddress = "0x0000000105705000"
+        image.imageVmAddress = "0x0000000105705000"
+        image.codeFile = "codeFile"
+        image.debugID = "debugID"
+        image.imageSize = 100
+        image.type = "macho"
+        
+        let debugImageProvider = TestDebugImageProvider()
+        debugImageProvider.debugImages = [image]
+        SentryDependencyContainer.sharedInstance().debugImageProvider = debugImageProvider
+        
         mockMetrics = MockMetric()
         systemWrapper.overrides.cpuUsage = mockMetrics.cpuUsage
         systemWrapper.overrides.memoryFootprintBytes = mockMetrics.memoryFootprint
diff --git a/Tests/SentryTests/Helper/TestDebugImageProvider.swift b/Tests/SentryTests/Helper/TestDebugImageProvider.swift
diff --git a/Tests/SentryTests/PrivateSentrySDKOnlyTests.swift b/Tests/SentryTests/PrivateSentrySDKOnlyTests.swift
diff --git a/Tests/SentryTests/SentryBinaryImageCacheTests.swift b/Tests/SentryTests/SentryBinaryImageCacheTests.swift
diff --git a/Tests/SentryTests/SentryCrash/SentryDebugImageProviderTests.swift b/Tests/SentryTests/SentryCrash/SentryDebugImageProviderTests.swift
diff --git a/Tests/SentryTests/Transaction/SentryTracerTests.swift b/Tests/SentryTests/Transaction/SentryTracerTests.swift

Original file line number	Diff line number	Diff line change
`@@ -60,8 +60,11 @@ + (nullable SentryEnvelope )envelopeWithData:(NSData )data`
`60`	`60`
`61`	`61`	`+ (NSArray<SentryDebugMeta > )getDebugImagesCrashed:(BOOL)isCrash`
`62`	`62`	`{`
	`63`	`+#pragma clang diagnostic push`
	`64`	`+#pragma clang diagnostic ignored "-Wdeprecated-declarations"`
`63`	`65`	`return [[SentryDependencyContainer sharedInstance].debugImageProvider`
`64`	`66`	`getDebugImagesCrashed:isCrash];`
	`67`	`+#pragma clang diagnostic pop`
`65`	`68`	`}`
`66`	`69`
`67`	`70`	`+ (nullable SentryAppStartMeasurement *)appStartMeasurement`
Original file line number	Diff line number	Diff line change
`@@ -863,7 +863,7 @@ - (void)addAppStartMeasurements:(SentryTransaction *)transaction`
`863`	`863`
`864`	`864`	`// The backend calculates statistics on the number and size of debug images for app`
`865`	`865`	`// start transactions. Therefore, we add all debug images here.`
`866`		`- transaction.debugMeta = [self.debugImageProvider getDebugImagesCrashed:NO];`
	`866`	`+ transaction.debugMeta = [self.debugImageProvider getDebugImagesFromCache];`
`867`	`867`	`}`
`868`	`868`	`}`
`869`	`869`	`}`