Skip to content

Commit

Permalink
impr: Speed up getBinaryImages
Browse files Browse the repository at this point in the history
Add two new internal methods getDebugImagesFromCacheForFrames
getDebugImagesFromCacheFrames to the SentryDebugImageProvider which use
the significantly faster SentryBinaryImageCache.

Fixes GH-4399
  • Loading branch information
philipphofmann committed Oct 14, 2024
1 parent 5a6e387 commit edb149c
Show file tree
Hide file tree
Showing 22 changed files with 375 additions and 35 deletions.
2 changes: 2 additions & 0 deletions .github/workflows/testflight.yml
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ on:
paths:
- '.github/workflows/testflight.yml'

# Only to trigger testflight upload for testing. Remove before merging.

jobs:
upload_to_testflight:
name: Build and Upload iOS-Swift to Testflight
Expand Down
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ via the option `swizzleClassNameExclude`.

- Serializing profile on a BG Thread (#4377) to avoid potentially slightly blocking the main thread.
- Session Replay performance for SwiftUI (#4419)
- Speed up getBinaryImages (#4435) for finishing transactions and capturing events

## 8.38.0-beta.1

Expand Down
4 changes: 4 additions & 0 deletions Sentry.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@
624688192C048EF10006179C /* SentryBaggageSerialization.swift in Sources */ = {isa = PBXBuildFile; fileRef = 624688182C048EF10006179C /* SentryBaggageSerialization.swift */; };
626E2D4C2BEA0C37005596FE /* SentryEnabledFeaturesBuilderTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 626E2D4B2BEA0C37005596FE /* SentryEnabledFeaturesBuilderTests.swift */; };
6271ADF32BA06D9B0098D2E9 /* SentryInternalSerializable.h in Headers */ = {isa = PBXBuildFile; fileRef = 6271ADF22BA06D9B0098D2E9 /* SentryInternalSerializable.h */; };
6273513F2CBD14970021D100 /* SentryDebugImageProvider+Private.h in Headers */ = {isa = PBXBuildFile; fileRef = 6273513E2CBD14970021D100 /* SentryDebugImageProvider+Private.h */; };
627E7589299F6FE40085504D /* SentryInternalDefines.h in Headers */ = {isa = PBXBuildFile; fileRef = 627E7588299F6FE40085504D /* SentryInternalDefines.h */; };
62862B1C2B1DDBC8009B16E3 /* SentryDelayedFrame.h in Headers */ = {isa = PBXBuildFile; fileRef = 62862B1B2B1DDBC8009B16E3 /* SentryDelayedFrame.h */; };
62862B1E2B1DDC35009B16E3 /* SentryDelayedFrame.m in Sources */ = {isa = PBXBuildFile; fileRef = 62862B1D2B1DDC35009B16E3 /* SentryDelayedFrame.m */; };
Expand Down Expand Up @@ -1081,6 +1082,7 @@
624688182C048EF10006179C /* SentryBaggageSerialization.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SentryBaggageSerialization.swift; sourceTree = "<group>"; };
626E2D4B2BEA0C37005596FE /* SentryEnabledFeaturesBuilderTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SentryEnabledFeaturesBuilderTests.swift; sourceTree = "<group>"; };
6271ADF22BA06D9B0098D2E9 /* SentryInternalSerializable.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; name = SentryInternalSerializable.h; path = include/SentryInternalSerializable.h; sourceTree = "<group>"; };
6273513E2CBD14970021D100 /* SentryDebugImageProvider+Private.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; name = "SentryDebugImageProvider+Private.h"; path = "include/SentryDebugImageProvider+Private.h"; sourceTree = "<group>"; };
627E7588299F6FE40085504D /* SentryInternalDefines.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; name = SentryInternalDefines.h; path = include/SentryInternalDefines.h; sourceTree = "<group>"; };
62862B1B2B1DDBC8009B16E3 /* SentryDelayedFrame.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; name = SentryDelayedFrame.h; path = include/SentryDelayedFrame.h; sourceTree = "<group>"; };
62862B1D2B1DDC35009B16E3 /* SentryDelayedFrame.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = SentryDelayedFrame.m; sourceTree = "<group>"; };
Expand Down Expand Up @@ -2317,6 +2319,7 @@
7BA61CB8247BC57B00C130A8 /* SentryCrashDefaultBinaryImageProvider.h */,
7BA61CBA247BC5D800C130A8 /* SentryCrashDefaultBinaryImageProvider.m */,
7BA61CAA247BA98100C130A8 /* SentryDebugImageProvider.h */,
6273513E2CBD14970021D100 /* SentryDebugImageProvider+Private.h */,
7BA61CAC247BAA0B00C130A8 /* SentryDebugImageProvider.m */,
7BA61CBE247CEA8100C130A8 /* SentryFormatter.h */,
7BA61CC7247D125400C130A8 /* SentryThreadInspector.h */,
Expand Down Expand Up @@ -4140,6 +4143,7 @@
7DC8310A2398283C0043DD9A /* SentryCrashIntegration.h in Headers */,
63FE718320DA4C1100CDBAE8 /* SentryCrashReportFixer.h in Headers */,
03F84D2027DD414C008FE43F /* SentryStackBounds.hpp in Headers */,
6273513F2CBD14970021D100 /* SentryDebugImageProvider+Private.h in Headers */,
8E5D38E3261D4B57000D363D /* SentryPerformanceTrackingIntegration.h in Headers */,
63FE70F920DA4C1000CDBAE8 /* SentryCrashMonitor.h in Headers */,
D8BD2E6829361A0F00D96C6A /* PrivatesHeader.h in Headers */,
Expand Down
3 changes: 2 additions & 1 deletion SentryTestUtils/TestClient.swift
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,14 @@ public class TestClient: SentryClient {

// Without this override we get a fatal error: use of unimplemented initializer
// see https://stackoverflow.com/questions/28187261/ios-swift-fatal-error-use-of-unimplemented-initializer-init
public override init(options: Options, transportAdapter: SentryTransportAdapter, fileManager: SentryFileManager, deleteOldEnvelopeItems: Bool, threadInspector: SentryThreadInspector, random: SentryRandomProtocol, locale: Locale, timezone: TimeZone) {
public override init(options: Options, transportAdapter: SentryTransportAdapter, fileManager: SentryFileManager, deleteOldEnvelopeItems: Bool, threadInspector: SentryThreadInspector, debugImageProvider: SentryDebugImageProvider, random: SentryRandomProtocol, locale: Locale, timezone: TimeZone) {
super.init(
options: options,
transportAdapter: transportAdapter,
fileManager: fileManager,
deleteOldEnvelopeItems: false,
threadInspector: threadInspector,
debugImageProvider: debugImageProvider,
random: random,
locale: locale,
timezone: timezone
Expand Down
14 changes: 14 additions & 0 deletions Sources/Sentry/SentryBinaryImageCache.m
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
#import "SentryBinaryImageCache.h"
#import "SentryCrashBinaryImageCache.h"
#include "SentryCrashUUIDConversion.h"
#import "SentryDependencyContainer.h"
#import "SentryInAppLogic.h"
#import "SentryLog.h"
Expand Down Expand Up @@ -59,7 +60,9 @@ - (void)binaryImageAdded:(const SentryCrashBinaryImage *)image

SentryBinaryImageInfo *newImage = [[SentryBinaryImageInfo alloc] init];
newImage.name = imageName;
newImage.UUID = [SentryBinaryImageCache convertUUID:image->uuid];
newImage.address = image->address;
newImage.vmAddress = image->vmAddress;
newImage.size = image->size;

@synchronized(self) {
Expand All @@ -80,6 +83,17 @@ - (void)binaryImageAdded:(const SentryCrashBinaryImage *)image
}
}

+ (NSString *_Nullable)convertUUID:(const unsigned char *const)value
{
if (nil == value) {
return nil;
}

char uuidBuffer[37];
sentrycrashdl_convertBinaryImageUUID(value, uuidBuffer);
return [[NSString alloc] initWithCString:uuidBuffer encoding:NSASCIIStringEncoding];
}

- (void)binaryImageRemoved:(const SentryCrashBinaryImage *)image
{
if (image == NULL) {
Expand Down
10 changes: 6 additions & 4 deletions Sources/Sentry/SentryClient.m
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
#import "SentryCrashDefaultMachineContextWrapper.h"
#import "SentryCrashIntegration.h"
#import "SentryCrashStackEntryMapper.h"
#import "SentryDebugImageProvider.h"
#import "SentryDebugImageProvider+Private.h"
#import "SentryDependencyContainer.h"
#import "SentryDispatchQueueWrapper.h"
#import "SentryDsn.h"
Expand Down Expand Up @@ -128,6 +128,7 @@ - (instancetype)initWithOptions:(SentryOptions *)options
fileManager:fileManager
deleteOldEnvelopeItems:deleteOldEnvelopeItems
threadInspector:threadInspector
debugImageProvider:[SentryDependencyContainer sharedInstance].debugImageProvider
random:[SentryDependencyContainer sharedInstance].random
locale:[NSLocale autoupdatingCurrentLocale]
timezone:[NSCalendar autoupdatingCurrentCalendar].timeZone];
Expand All @@ -138,6 +139,7 @@ - (instancetype)initWithOptions:(SentryOptions *)options
fileManager:(SentryFileManager *)fileManager
deleteOldEnvelopeItems:(BOOL)deleteOldEnvelopeItems
threadInspector:(SentryThreadInspector *)threadInspector
debugImageProvider:(SentryDebugImageProvider *)debugImageProvider
random:(id<SentryRandom>)random
locale:(NSLocale *)locale
timezone:(NSTimeZone *)timezone
Expand All @@ -149,7 +151,7 @@ - (instancetype)initWithOptions:(SentryOptions *)options
self.fileManager = fileManager;
self.threadInspector = threadInspector;
self.random = random;
self.debugImageProvider = [SentryDependencyContainer sharedInstance].debugImageProvider;
self.debugImageProvider = debugImageProvider;
self.locale = locale;
self.timezone = timezone;
self.attachmentProcessors = [[NSMutableArray alloc] init];
Expand Down Expand Up @@ -688,8 +690,8 @@ - (SentryEvent *_Nullable)prepareEvent:(SentryEvent *)event
BOOL debugMetaNotAttached = !(nil != event.debugMeta && event.debugMeta.count > 0);
if (!isCrashEvent && shouldAttachStacktrace && debugMetaNotAttached
&& event.threads != nil) {
event.debugMeta = [self.debugImageProvider getDebugImagesForThreads:event.threads
isCrash:NO];
event.debugMeta =
[self.debugImageProvider getDebugImagesFromCacheForThreads:event.threads];
}
}

Expand Down
72 changes: 59 additions & 13 deletions Sources/Sentry/SentryDebugImageProvider.m
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
#import "SentryDebugImageProvider.h"
#import "SentryBinaryImageCache.h"
#import "SentryCrashDefaultBinaryImageProvider.h"
#import "SentryCrashDynamicLinker.h"
#import "SentryCrashUUIDConversion.h"
#import "SentryDebugMeta.h"
#import "SentryDependencyContainer.h"
#import "SentryFormatter.h"
#import "SentryFrame.h"
#import "SentryInternalDefines.h"
Expand All @@ -14,6 +16,8 @@
@interface SentryDebugImageProvider ()

@property (nonatomic, strong) id<SentryCrashBinaryImageProvider> binaryImageProvider;
@property (nonatomic, strong) SentryBinaryImageCache *binaryImageCache;

@end

@implementation SentryDebugImageProvider
Expand All @@ -23,16 +27,20 @@ - (instancetype)init
SentryCrashDefaultBinaryImageProvider *provider =
[[SentryCrashDefaultBinaryImageProvider alloc] init];

self = [self initWithBinaryImageProvider:provider];
self = [self
initWithBinaryImageProvider:provider
binaryImageCache:SentryDependencyContainer.sharedInstance.binaryImageCache];

return self;
}

/** Internal constructor for testing */
- (instancetype)initWithBinaryImageProvider:(id<SentryCrashBinaryImageProvider>)binaryImageProvider
binaryImageCache:(SentryBinaryImageCache *)binaryImageCache
{
if (self = [super init]) {
self.binaryImageProvider = binaryImageProvider;
self.binaryImageCache = binaryImageCache;
}
return self;
}
Expand Down Expand Up @@ -95,6 +103,55 @@ - (void)extractDebugImageAddressFromFrames:(NSArray<SentryFrame *> *)frames
return [self getDebugImagesForAddresses:imageAddresses isCrash:isCrash];
}

- (NSArray<SentryDebugMeta *> *)getDebugImagesFromCacheForFrames:(NSArray<SentryFrame *> *)frames
{
NSMutableSet<NSString *> *imageAddresses = [[NSMutableSet alloc] init];
[self extractDebugImageAddressFromFrames:frames intoSet:imageAddresses];

return [self getDebugImagesForImageAddressesFromCache:imageAddresses];
}

- (NSArray<SentryDebugMeta *> *)getDebugImagesFromCacheForThreads:(NSArray<SentryThread *> *)threads
{
NSMutableSet<NSString *> *imageAddresses = [[NSMutableSet alloc] init];

for (SentryThread *thread in threads) {
[self extractDebugImageAddressFromFrames:thread.stacktrace.frames intoSet:imageAddresses];
}

return [self getDebugImagesForImageAddressesFromCache:imageAddresses];
}

- (NSArray<SentryDebugMeta *> *)getDebugImagesForImageAddressesFromCache:
(NSSet<NSString *> *)imageAddresses
{
NSMutableArray<SentryDebugMeta *> *result = [NSMutableArray array];

for (NSString *imageAddress in imageAddresses) {
const uint64_t imageAddressAsUInt64 = sentry_UInt64ForHexAddress(imageAddress);
SentryBinaryImageInfo *info = [self.binaryImageCache imageByAddress:imageAddressAsUInt64];
if (info == nil) {
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];
}

return result;
}

- (NSArray<SentryDebugMeta *> *)getDebugImages
{
// maintains previous behavior for the same method call by also trying to gather crash info
Expand All @@ -118,7 +175,7 @@ - (void)extractDebugImageAddressFromFrames:(NSArray<SentryFrame *> *)frames
- (SentryDebugMeta *)fillDebugMetaFrom:(SentryCrashBinaryImage)image
{
SentryDebugMeta *debugMeta = [[SentryDebugMeta alloc] init];
debugMeta.debugID = [SentryDebugImageProvider convertUUID:image.uuid];
debugMeta.debugID = [SentryBinaryImageCache convertUUID:image.uuid];
debugMeta.type = SentryDebugImageType;

if (image.vmAddress > 0) {
Expand All @@ -136,15 +193,4 @@ - (SentryDebugMeta *)fillDebugMetaFrom:(SentryCrashBinaryImage)image
return debugMeta;
}

+ (NSString *_Nullable)convertUUID:(const unsigned char *const)value
{
if (nil == value) {
return nil;
}

char uuidBuffer[37];
sentrycrashdl_convertBinaryImageUUID(value, uuidBuffer);
return [[NSString alloc] initWithCString:uuidBuffer encoding:NSASCIIStringEncoding];
}

@end
11 changes: 10 additions & 1 deletion Sources/Sentry/SentryDependencyContainer.m
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,6 @@ - (instancetype)init
_random = [[SentryRandom alloc] init];
_threadWrapper = [[SentryThreadWrapper alloc] init];
_binaryImageCache = [[SentryBinaryImageCache alloc] init];
_debugImageProvider = [[SentryDebugImageProvider alloc] init];
_dateProvider = [[SentryCurrentDateProvider alloc] init];
}
return self;
Expand Down Expand Up @@ -183,6 +182,16 @@ - (SentryThreadInspector *)threadInspector SENTRY_DISABLE_THREAD_SANITIZER(
return _threadInspector;
}

- (SentryDebugImageProvider *)debugImageProvider
{
@synchronized(sentryDependencyContainerLock) {
if (_debugImageProvider == nil) {
_debugImageProvider = [[SentryDebugImageProvider alloc] init];
}
return _debugImageProvider;
}
}

- (SentryExtraContextProvider *)extraContextProvider SENTRY_DISABLE_THREAD_SANITIZER(
"double-checked lock produce false alarms")
{
Expand Down
1 change: 0 additions & 1 deletion Sources/Sentry/SentryThreadInspector.m
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
#import "SentryThreadInspector.h"
#import "SentryBinaryImageCache.h"
#import "SentryCrashDefaultMachineContextWrapper.h"
#import "SentryCrashStackCursor.h"
#include "SentryCrashStackCursor_MachineContext.h"
Expand Down
6 changes: 3 additions & 3 deletions Sources/Sentry/SentryTracer.m
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
#import "PrivateSentrySDKOnly.h"
#import "SentryClient.h"
#import "SentryDebugImageProvider.h"
#import "SentryDebugImageProvider+Private.h"
#import "SentryDependencyContainer.h"
#import "SentryEvent+Private.h"
#import "SentryFileManager.h"
Expand Down Expand Up @@ -717,8 +717,8 @@ - (SentryTransaction *)toTransaction
if (framesOfAllSpans.count > 0) {
SentryDebugImageProvider *debugImageProvider
= SentryDependencyContainer.sharedInstance.debugImageProvider;
transaction.debugMeta = [debugImageProvider getDebugImagesForFrames:framesOfAllSpans
isCrash:NO];
transaction.debugMeta =
[debugImageProvider getDebugImagesFromCacheForFrames:framesOfAllSpans];
}

#if SENTRY_HAS_UIKIT
Expand Down
5 changes: 5 additions & 0 deletions Sources/Sentry/include/HybridPublic/SentryBinaryImageCache.h
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,11 @@ NS_ASSUME_NONNULL_BEGIN

@interface SentryBinaryImageInfo : NSObject
@property (nonatomic, strong) NSString *name;
@property (nonatomic, copy) NSString *UUID;
@property (nonatomic) uint64_t vmAddress;
@property (nonatomic) uint64_t address;
@property (nonatomic) uint64_t size;

@end

/**
Expand All @@ -23,6 +26,8 @@ NS_ASSUME_NONNULL_BEGIN

- (nullable NSString *)pathForInAppInclude:(NSString *)inAppInclude;

+ (NSString *_Nullable)convertUUID:(const unsigned char *const)value;

@end

NS_ASSUME_NONNULL_END
13 changes: 13 additions & 0 deletions Sources/Sentry/include/HybridPublic/SentryFormatter.h
Original file line number Diff line number Diff line change
Expand Up @@ -41,3 +41,16 @@ sentry_formatHexAddressUInt64(uint64_t value)
{
return sentry_snprintfHexAddress(value);
}

static inline uint64_t
sentry_UInt64ForHexAddress(NSString *hexString)
{
uint64_t value = 0;
NSScanner *scanner = [NSScanner scannerWithString:hexString];

if ([scanner scanHexLongLong:&value]) {
return value;
} else {
return 0;
}
}
29 changes: 29 additions & 0 deletions Sources/Sentry/include/SentryDebugImageProvider+Private.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
#import "SentryDebugImageProvider.h"

@class SentryDebugMeta;
@class SentryThread;
@class SentryFrame;

NS_ASSUME_NONNULL_BEGIN

@interface SentryDebugImageProvider ()

/**
* Returns a list of debug images that are being referenced by the given frames.
* This function uses the @c SentryBinaryImageCache which is significantly faster than @c
* SentryCrashDefaultBinaryImageProvider for retrieving binary image information.
*/
- (NSArray<SentryDebugMeta *> *)getDebugImagesFromCacheForFrames:(NSArray<SentryFrame *> *)frames
NS_SWIFT_NAME(getDebugImagesFromCacheForFrames(frames:));

/**
* Returns a list of debug images that are being referenced in the given threads.
* This function uses the @c SentryBinaryImageCache which is significantly faster than @c
* SentryCrashDefaultBinaryImageProvider for retrieving binary image information.
*/
- (NSArray<SentryDebugMeta *> *)getDebugImagesFromCacheForThreads:(NSArray<SentryThread *> *)threads
NS_SWIFT_NAME(getDebugImagesFromCacheForThreads(threads:));

@end

NS_ASSUME_NONNULL_END
Loading

0 comments on commit edb149c

Please sign in to comment.