impr: Speed up getBinaryImages #4435

philipphofmann · 2024-10-14T09:42:25Z

📜 Description

Add two new internal methods getDebugImagesFromCacheForFrames and getDebugImagesFromCacheForThreads to the SentryDebugImageProvider, which use the significantly faster SentryBinaryImageCache.

sentry-cocoa/Sources/Sentry/include/HybridPublic/SentryBinaryImageCache.h

Lines 11 to 16 in 5a6e387

    
           /** 
        
            * This class listens to `SentryCrashBinaryImageCache` to keep a copy of the loaded binaries 
        
            * information in a sorted collection that will be used to symbolicate frames with better 
        
            * performance. 
        
            */ 
        
           @interface SentryBinaryImageCache : NSObject

💡 Motivation and Context

Fixes GH-4399

💚 How did you test it?

Will test it with TestFlight builds.

Main Branch

Event in Sentry

Debug Meta Event JSON

{
  "code_file": "/private/var/containers/Bundle/Application/62D5E98F-1727-439B-AEC8-6DD71F97B71D/iOS-Swift.app/iOS-Swift",
  "debug_id": "289a8392-886c-302a-991e-3577820b1c48",
  "arch": "arm64",
  "image_addr": "0x104e58000",
  "image_size": 262144,
  "image_vmaddr": "0x100000000",
  "debug_status": "found",
  "type": "macho"
},
{
  "code_file": "/private/var/containers/Bundle/Application/62D5E98F-1727-439B-AEC8-6DD71F97B71D/iOS-Swift.app/Frameworks/Sentry.framework/Sentry",
  "debug_id": "0889ec9f-e6bb-3a0e-b670-122b9290c1bf",
  "arch": "arm64",
  "image_addr": "0x1052b4000",
  "image_size": 1196032,
  "debug_status": "found",
  "type": "macho"
},
{
  "code_file": "/System/Library/Frameworks/CoreFoundation.framework/CoreFoundation",
  "debug_id": "4250d45c-51a9-3ac5-8c3d-69de936fad46",
  "arch": "arm64e",
  "image_addr": "0x199251000",
  "image_size": 5529600,
  "image_vmaddr": "0x182d95000",
  "debug_status": "found",
  "type": "macho"
},
{
  "code_file": "/System/Library/PrivateFrameworks/GraphicsServices.framework/GraphicsServices",
  "debug_id": "227122eb-72fd-3ecb-9f66-986addd8e048",
  "arch": "arm64e",
  "image_addr": "0x1e4d39000",
  "image_size": 36864,
  "image_vmaddr": "0x1ce87d000",
  "debug_status": "found",
  "type": "macho"
},
{
  "code_file": "/System/Library/PrivateFrameworks/UIKitCore.framework/UIKitCore",
  "debug_id": "187c890e-8e4c-32f2-a97e-fa905a55f094",
  "arch": "arm64e",
  "image_addr": "0x19ba1c000",
  "image_size": 32247808,
  "image_vmaddr": "0x185560000",
  "debug_status": "found",
  "type": "macho"
}

Transaction in Sentry

Transaction Debug Meta JSON

{
  "code_file": "/Users/philipp.hofmann/Library/Developer/CoreSimulator/Devices/479CAA0E-3CC9-4B32-BBA2-B6A9E85F1C65/data/Containers/Bundle/Application/CC287D18-AEA8-4E13-B474-BBED1E625104/iOS-Swift.app/iOS-Swift.debug.dylib",
  "debug_id": "30c91ede-2cad-3e55-a6e6-8ef5d437bcde",
  "image_addr": "0x10501c000",
  "image_size": 475136,
  "type": "macho"
}

Feature Branch

Event in Sentry

Debug Meta Event JSON

{
  "code_file": "/private/var/containers/Bundle/Application/90A1CD24-E825-411B-A3CB-9E6796A3EF74/iOS-Swift.app/Frameworks/Sentry.framework/Sentry",
  "debug_id": "0ca81d91-985b-3ede-81e1-a868f53b4cce",
  "arch": "arm64",
  "image_addr": "0x10534c000",
  "image_size": 1196032,
  "debug_status": "found",
  "type": "macho"
},
{
  "code_file": "/System/Library/Frameworks/CoreFoundation.framework/CoreFoundation",
  "debug_id": "4250d45c-51a9-3ac5-8c3d-69de936fad46",
  "arch": "arm64e",
  "image_addr": "0x199251000",
  "image_size": 5529600,
  "image_vmaddr": "0x182d95000",
  "debug_status": "found",
  "type": "macho"
},
{
  "code_file": "/System/Library/PrivateFrameworks/GraphicsServices.framework/GraphicsServices",
  "debug_id": "227122eb-72fd-3ecb-9f66-986addd8e048",
  "arch": "arm64e",
  "image_addr": "0x1e4d39000",
  "image_size": 36864,
  "image_vmaddr": "0x1ce87d000",
  "debug_status": "found",
  "type": "macho"
},
{
  "code_file": "/private/var/containers/Bundle/Application/90A1CD24-E825-411B-A3CB-9E6796A3EF74/iOS-Swift.app/iOS-Swift",
  "debug_id": "289a8392-886c-302a-991e-3577820b1c48",
  "arch": "arm64",
  "image_addr": "0x104e98000",
  "image_size": 262144,
  "image_vmaddr": "0x100000000",
  "debug_status": "found",
  "type": "macho"
},
{
  "code_file": "/System/Library/PrivateFrameworks/UIKitCore.framework/UIKitCore",
  "debug_id": "187c890e-8e4c-32f2-a97e-fa905a55f094",
  "arch": "arm64e",
  "image_addr": "0x19ba1c000",
  "image_size": 32247808,
  "image_vmaddr": "0x185560000",
  "debug_status": "found",
  "type": "macho"
}

Transaction in Sentry

Transaction Debug Meta JSON

{
  "code_file": "/Users/philipp.hofmann/Library/Developer/CoreSimulator/Devices/479CAA0E-3CC9-4B32-BBA2-B6A9E85F1C65/data/Containers/Bundle/Application/2EA49ABC-0881-420B-9533-E0A962158479/iOS-Swift.app/iOS-Swift.debug.dylib",
  "debug_id": "30c91ede-2cad-3e55-a6e6-8ef5d437bcde",
  "image_addr": "0x1010a0000",
  "image_size": 475136,
  "type": "macho"
}

Profiles

Main Branch

Feature Branch

The profile of the feature branch doesn't show anything suspicious for NSScanner, which the sentry_UInt64ForHexAddress uses. The profile on the main branch highlights that sentrycrashdl_getBinaryImageForHeader has 2.00 Mc and weight of 0.4%. The getDebugImagesForThreads has a weight of 2,0% on the main branch, but the getDebugImagesFromCacheForThreads has only 0.3% weight. This proves that the new implementation of getDebugImagesFromCacheForThreads should be significantly faster.

📝 Checklist

You have to check all boxes before merging:

I reviewed the submitted code.
I added tests to verify the changes.
No new PII added or SDK only sends newly added PII if sendDefaultPII is enabled.
I updated the docs if needed.
Review from the native team if needed.
No breaking change or entry added to the changelog.
No breaking change for hybrid SDKs or communicated to hybrid SDKs.

🔮 Next steps

Add two new internal methods getDebugImagesFromCacheForFrames getDebugImagesFromCacheFrames to the SentryDebugImageProvider which use the significantly faster SentryBinaryImageCache. Fixes GH-4399

Sources/Sentry/SentryBinaryImageCache.m

codecov · 2024-10-14T09:50:03Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 91.333%. Comparing base (5a6e387) to head (641c1e1).
Report is 2 commits behind head on main.

Additional details and impacted files

@@              Coverage Diff              @@
##              main     #4435       +/-   ##
=============================================
+ Coverage   91.319%   91.333%   +0.013%     
=============================================
  Files          610       610               
  Lines        49755     49926      +171     
  Branches     17928     18037      +109     
=============================================
+ Hits         45436     45599      +163     
- Misses        4227      4234        +7     
- Partials        92        93        +1

Files with missing lines	Coverage Δ
SentryTestUtils/TestClient.swift	`85.950% <100.000%> (+0.117%)`	⬆️
Sources/Sentry/SentryBinaryImageCache.m	`100.000% <100.000%> (ø)`
Sources/Sentry/SentryClient.m	`98.594% <100.000%> (+0.002%)`	⬆️
Sources/Sentry/SentryDebugImageProvider.m	`91.304% <100.000%> (-2.446%)`	⬇️
Sources/Sentry/SentryDependencyContainer.m	`96.319% <100.000%> (+0.069%)`	⬆️
Sources/Sentry/SentryThreadInspector.m	`99.009% <ø> (ø)`
Sources/Sentry/SentryTracer.m	`96.912% <100.000%> (-0.951%)`	⬇️
...rces/Sentry/include/HybridPublic/SentryFormatter.h	`73.076% <100.000%> (+8.076%)`	⬆️
...ests/SentryTests/Helper/SentryFormatterTests.swift	`100.000% <100.000%> (ø)`
...ts/SentryTests/Helper/TestDebugImageProvider.swift	`83.333% <100.000%> (+33.333%)`	⬆️
... and 4 more

... and 2 files with indirect coverage changes

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5a6e387...641c1e1. Read the comment docs.

github-actions · 2024-10-14T09:59:43Z

Performance metrics 🚀

	Plain	With Sentry	Diff
Startup time	1245.66 ms	1257.58 ms	11.92 ms
Size	21.58 KiB	707.35 KiB	685.77 KiB

Baseline results on branch: main

Startup times

Revision	Plain	With Sentry	Diff
`7cd187e`	1239.02 ms	1261.42 ms	22.40 ms
`9fa5d27`	1230.65 ms	1255.08 ms	24.43 ms
`f1ed6f8`	1210.94 ms	1230.78 ms	19.84 ms
`5b888cb`	1218.28 ms	1244.65 ms	26.37 ms
`8aec30e`	1235.73 ms	1255.87 ms	20.14 ms
`326b7eb`	1223.41 ms	1235.66 ms	12.25 ms
`1c0e955`	1275.71 ms	1305.69 ms	29.98 ms
`5e769dd`	1216.24 ms	1245.74 ms	29.50 ms
`282cc99`	1232.59 ms	1245.88 ms	13.29 ms
`8f397a7`	1196.55 ms	1226.82 ms	30.27 ms

App size

Revision	Plain	With Sentry	Diff
`7cd187e`	20.76 KiB	401.65 KiB	380.89 KiB
`9fa5d27`	20.76 KiB	393.37 KiB	372.61 KiB
`f1ed6f8`	21.58 KiB	683.51 KiB	661.93 KiB
`5b888cb`	21.58 KiB	654.67 KiB	633.09 KiB
`8aec30e`	21.58 KiB	616.75 KiB	595.17 KiB
`326b7eb`	20.76 KiB	432.31 KiB	411.55 KiB
`1c0e955`	21.58 KiB	424.28 KiB	402.70 KiB
`5e769dd`	21.58 KiB	572.21 KiB	550.62 KiB
`282cc99`	22.85 KiB	414.09 KiB	391.24 KiB
`8f397a7`	20.76 KiB	420.55 KiB	399.79 KiB

Previous results on branch: impr/speed-up-get-debug-images

Startup times

Revision	Plain	With Sentry	Diff
`5096089`	1214.55 ms	1239.74 ms	25.19 ms
`cb282da`	1237.39 ms	1259.47 ms	22.08 ms

App size

Revision	Plain	With Sentry	Diff
`5096089`	21.58 KiB	707.35 KiB	685.77 KiB
`cb282da`	21.58 KiB	707.35 KiB	685.77 KiB

brustolin

LGTM

.github/workflows/testflight.yml

Sources/Sentry/SentryDebugImageProvider.m

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.

impr: Speed up getBinaryImages

edb149c

Add two new internal methods getDebugImagesFromCacheForFrames getDebugImagesFromCacheFrames to the SentryDebugImageProvider which use the significantly faster SentryBinaryImageCache. Fixes GH-4399

philipphofmann requested review from armcknight and brustolin as code owners October 14, 2024 09:42

philipphofmann commented Oct 14, 2024

View reviewed changes

Sources/Sentry/SentryBinaryImageCache.m Show resolved Hide resolved

philipphofmann marked this pull request as draft October 14, 2024 09:47

philipphofmann added 2 commits October 14, 2024 12:36

fix tests on macos

d8d51e7

undo testflight

625b194

philipphofmann marked this pull request as ready for review October 14, 2024 11:18

brustolin approved these changes Oct 14, 2024

View reviewed changes

.github/workflows/testflight.yml Outdated Show resolved Hide resolved

krystofwoldrich reviewed Oct 14, 2024

View reviewed changes

Sources/Sentry/SentryDebugImageProvider.m Show resolved Hide resolved

expose for hybrid SDKs

641c1e1

This was referenced Oct 14, 2024

Use cached methods SentryDebugImageProvider of the Cocoa SDK getsentry/sentry-react-native#4169

Closed

Only send debug images referenced in the stacktrace for events getsentry/sentry-dart#1755

Closed

philipphofmann merged commit a2cf26e into main Oct 14, 2024
65 checks passed

philipphofmann deleted the impr/speed-up-get-debug-images branch October 14, 2024 13:06

antonis mentioned this pull request Oct 23, 2024

Use cached methods SentryDebugImageProvider of the Cocoa SDK getsentry/sentry-react-native#4194

Merged

9 tasks

philipphofmann mentioned this pull request Nov 15, 2024

impr: Speed up getBinaryImages V2 #4539

Merged

7 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

impr: Speed up getBinaryImages #4435

impr: Speed up getBinaryImages #4435

Uh oh!

philipphofmann commented Oct 14, 2024 •

edited

Loading

Uh oh!

Uh oh!

codecov bot commented Oct 14, 2024 •

edited

Loading

Uh oh!

github-actions bot commented Oct 14, 2024 •

edited

Loading

Baseline results on branch: main

Startup times

App size

Previous results on branch: impr/speed-up-get-debug-images

Startup times

App size

Uh oh!

brustolin left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

	/**
	* This class listens to `SentryCrashBinaryImageCache` to keep a copy of the loaded binaries
	* information in a sorted collection that will be used to symbolicate frames with better
	* performance.
	*/
	@interface SentryBinaryImageCache : NSObject

Uh oh!

impr: Speed up getBinaryImages #4435

impr: Speed up getBinaryImages #4435

Uh oh!

Conversation

philipphofmann commented Oct 14, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

📜 Description

💡 Motivation and Context

💚 How did you test it?

Main Branch

Feature Branch

Profiles

📝 Checklist

🔮 Next steps

Uh oh!

Uh oh!

codecov bot commented Oct 14, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

github-actions bot commented Oct 14, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Performance metrics 🚀

Baseline results on branch: main

Startup times

App size

Previous results on branch: impr/speed-up-get-debug-images

Startup times

App size

Uh oh!

brustolin left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

philipphofmann commented Oct 14, 2024 •

edited

Loading

codecov bot commented Oct 14, 2024 •

edited

Loading

github-actions bot commented Oct 14, 2024 •

edited

Loading