Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

impr: Speed up getBinaryImages V2 #4539

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .github/workflows/testflight.yml
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ on:
push:
branches:
- main
- impr/speed-up-get-debug-images-v2

paths:
- 'Sources/**'
Expand Down
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,10 @@
- Finish TTFD when not calling reportFullyDisplayed before binding a new transaction to the scope (#4526).
- Session replay opacity animation masking (#4532)

### Improvements

- impr: Speed up getBinaryImages V2 (#4539). Follow up on (#4435)

## 8.40.1

### Fixes
Expand Down
6 changes: 3 additions & 3 deletions Sentry.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -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 */; };
Expand Down Expand Up @@ -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 */; };
Expand Down Expand Up @@ -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 */,
Expand Down Expand Up @@ -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 */,
Expand Down Expand Up @@ -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 */,
Expand Down Expand Up @@ -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 */,
Expand Down
2 changes: 2 additions & 0 deletions SentryTestUtils/SentryTestUtils-ObjC-BridgingHeader.h
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
36 changes: 36 additions & 0 deletions SentryTestUtils/TestDebugImageProvider.swift
Original file line number Diff line number Diff line change
@@ -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)
}

Check warning on line 11 in SentryTestUtils/TestDebugImageProvider.swift

View check run for this annotation

Codecov / codecov/patch

SentryTestUtils/TestDebugImageProvider.swift#L10-L11

Added lines #L10 - L11 were not covered by tests

@available(*, deprecated)
public override func getDebugImagesCrashed(_ isCrash: Bool) -> [DebugMeta] {
debugImages ?? super.getDebugImagesCrashed(isCrash)
}

Check warning on line 16 in SentryTestUtils/TestDebugImageProvider.swift

View check run for this annotation

Codecov / codecov/patch

SentryTestUtils/TestDebugImageProvider.swift#L16

Added line #L16 was not covered by tests

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()
}
}
3 changes: 3 additions & 0 deletions Sources/Sentry/PrivateSentrySDKOnly.mm
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
9 changes: 6 additions & 3 deletions Sources/Sentry/Profiling/SentryProfilerSerialization.mm
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

# import "SentryClient+Private.h"
# import "SentryDateUtils.h"
# import "SentryDebugImageProvider+HybridSDKs.h"
# import "SentryDependencyContainer.h"
# import "SentryDevice.h"
# import "SentryEnvelope.h"
Expand Down Expand Up @@ -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
,
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
15 changes: 12 additions & 3 deletions Sources/Sentry/Public/SentryDebugImageProvider.h
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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
Expand All @@ -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

Expand Down
11 changes: 11 additions & 0 deletions Sources/Sentry/SentryBinaryImageCache.m
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -155,6 +157,13 @@ - (NSInteger)indexOfImage:(uint64_t)address
return imagePaths;
}

- (NSArray<SentryBinaryImageInfo *> *)getAllBinaryImages
{
@synchronized(self) {
return _cache.copy;
}
}

@end

static void
Expand All @@ -168,3 +177,5 @@ - (NSInteger)indexOfImage:(uint64_t)address
{
[SentryDependencyContainer.sharedInstance.binaryImageCache binaryImageRemoved:image];
}

NS_ASSUME_NONNULL_END
49 changes: 36 additions & 13 deletions Sources/Sentry/SentryDebugImageProvider.m
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@
#import "SentryThread.h"
#import <Foundation/Foundation.h>

NS_ASSUME_NONNULL_BEGIN

@interface SentryDebugImageProvider ()

@property (nonatomic, strong) id<SentryCrashBinaryImageProvider> binaryImageProvider;
Expand Down Expand Up @@ -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]) {
Expand Down Expand Up @@ -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;
Expand All @@ -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];
Expand Down Expand Up @@ -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
2 changes: 1 addition & 1 deletion Sources/Sentry/SentryTracer.m
Original file line number Diff line number Diff line change
Expand Up @@ -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];
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
13 changes: 13 additions & 0 deletions Tests/SentryProfilerTests/SentryProfileTestFixture.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
27 changes: 0 additions & 27 deletions Tests/SentryTests/Helper/TestDebugImageProvider.swift

This file was deleted.

Loading
Loading