Skip to content

impr(profiling): always remove launch profile config after starting #5318

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

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

armcknight
Copy link
Member

@armcknight armcknight commented May 29, 2025

I realized after rereading #3637 (comment) that even if we consider a launch profile unable to be stopped as a misconfiguration we can't currently avoid at runtime, this leads to every launch afterwards also starting the launch profiling. We should instead consider not starting the SDK as disabling launch profiling, the same as if the SDK had never been started in an app.

To achieve this we should always remove the config file once we're done with it. Configuring to disable launch profiling already removed any file present; now, starting the launch profiler also removes it.

Leaving as a draft until I can write/modify a test for it.

#skip-changelog

Copy link

codecov bot commented May 29, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 92.898%. Comparing base (5e8f548) to head (8f837d5).

Additional details and impacted files

Impacted file tree graph

@@              Coverage Diff              @@
##              main     #5318       +/-   ##
=============================================
- Coverage   92.906%   92.898%   -0.009%     
=============================================
  Files          691       691               
  Lines        86703     86709        +6     
  Branches     31219     31239       +20     
=============================================
- Hits         80553     80551        -2     
- Misses        6050      6057        +7     
- Partials       100       101        +1     
Files with missing lines Coverage Δ
Sources/Sentry/Profiling/SentryLaunchProfiling.m 89.056% <100.000%> (+0.041%) ⬆️

... and 15 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 5e8f548...8f837d5. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Contributor

Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1224.88 ms 1236.13 ms 11.25 ms
Size 23.76 KiB 821.51 KiB 797.75 KiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
3437454 1225.64 ms 1234.31 ms 8.67 ms
f273312 1242.92 ms 1259.78 ms 16.86 ms
1a3ac75 1240.14 ms 1252.24 ms 12.10 ms
ee51b69 1229.12 ms 1246.45 ms 17.33 ms
f1b97be 1227.90 ms 1233.79 ms 5.89 ms
2ca242e 1228.19 ms 1244.78 ms 16.59 ms
66922ca 1221.68 ms 1235.98 ms 14.30 ms
49ca74d 1228.90 ms 1252.00 ms 23.10 ms
b066509 1246.14 ms 1272.42 ms 26.28 ms
1731a1c 1237.33 ms 1255.45 ms 18.12 ms

App size

Revision Plain With Sentry Diff
3437454 22.85 KiB 408.87 KiB 386.02 KiB
f273312 21.58 KiB 707.35 KiB 685.77 KiB
1a3ac75 23.76 KiB 821.43 KiB 797.67 KiB
ee51b69 22.30 KiB 832.29 KiB 809.98 KiB
f1b97be 21.58 KiB 681.74 KiB 660.16 KiB
2ca242e 21.58 KiB 643.27 KiB 621.69 KiB
66922ca 20.76 KiB 425.80 KiB 405.04 KiB
49ca74d 22.30 KiB 747.28 KiB 724.98 KiB
b066509 22.84 KiB 403.13 KiB 380.29 KiB
1731a1c 21.58 KiB 542.28 KiB 520.69 KiB

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant