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

ref(profiling): function renames #5007

Conversation

armcknight
Copy link
Member

@armcknight armcknight commented Mar 21, 2025

I was getting lost trying to debug launch profiling stuff so i renamed some things to try to help bring more clarity. Tried to keep this free of anything but renames, no logic should be changed.

#skip-changelog; for #4853

Copy link

github-actions bot commented Mar 21, 2025

🚨 Detected changes in high risk code 🚨

High-risk code can easily blow up and is hard to test. We had severe bugs in the past. Be extra careful when changing these files, and have an extra careful look at these:

  • Sources/Sentry/SentryFileManager.m

Copy link

github-actions bot commented Mar 21, 2025

Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1212.73 ms 1247.43 ms 34.70 ms
Size 22.30 KiB 843.09 KiB 820.78 KiB

Baseline results on branch: armcknight/git-chain/profiling/new-continuous-apis/6-launch-profiling

Startup times

Revision Plain With Sentry Diff
2fd0f0e 1225.94 ms 1247.47 ms 21.53 ms
9839a68 1228.24 ms 1247.98 ms 19.73 ms
cf7f7c2 1215.34 ms 1239.89 ms 24.55 ms
284b1c6 1245.83 ms 1264.37 ms 18.53 ms
0a0a2bf 1213.19 ms 1239.20 ms 26.01 ms
4f73298 1211.70 ms 1234.02 ms 22.32 ms

App size

Revision Plain With Sentry Diff
2fd0f0e 22.30 KiB 842.05 KiB 819.75 KiB
9839a68 22.30 KiB 843.83 KiB 821.52 KiB
cf7f7c2 22.30 KiB 842.86 KiB 820.56 KiB
284b1c6 22.30 KiB 843.83 KiB 821.52 KiB
0a0a2bf 22.30 KiB 843.82 KiB 821.52 KiB
4f73298 22.30 KiB 843.23 KiB 820.92 KiB

Previous results on branch: armcknight/profiling/new-continuous-apis/8-function-renames

Startup times

Revision Plain With Sentry Diff
d974391 1241.08 ms 1258.67 ms 17.59 ms
51eb583 1236.73 ms 1255.69 ms 18.96 ms
006c78d 1226.40 ms 1241.45 ms 15.05 ms
82f99cb 1237.65 ms 1260.96 ms 23.31 ms

App size

Revision Plain With Sentry Diff
d974391 22.30 KiB 843.77 KiB 821.47 KiB
51eb583 22.30 KiB 843.09 KiB 820.79 KiB
006c78d 22.30 KiB 843.78 KiB 821.48 KiB
82f99cb 22.30 KiB 843.72 KiB 821.41 KiB

Copy link

github-actions bot commented Mar 21, 2025

🚨 Detected changes in high risk code 🚨

High-risk code can easily blow up and is hard to test. We had severe bugs in the past. Be extra careful when changing these files, and have an extra careful look at these:

  • Sources/Sentry/SentryFileManager.m

Copy link

codecov bot commented Mar 21, 2025

Codecov Report

Attention: Patch coverage is 98.41270% with 1 line in your changes missing coverage. Please review.

Please upload report for BASE (armcknight/git-chain/profiling/new-continuous-apis/6-launch-profiling@b8d0e7d). Learn more about missing BASE report.

Files with missing lines Patch % Lines
Sources/Sentry/PrivateSentrySDKOnly.mm 0.000% 1 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@                                            Coverage Diff                                            @@
##             armcknight/git-chain/profiling/new-continuous-apis/6-launch-profiling     #5007   +/-   ##
=========================================================================================================
  Coverage                                                                         ?   92.743%           
=========================================================================================================
  Files                                                                            ?       674           
  Lines                                                                            ?     83279           
  Branches                                                                         ?     30350           
=========================================================================================================
  Hits                                                                             ?     77236           
  Misses                                                                           ?      5945           
  Partials                                                                         ?        98           
Files with missing lines Coverage Δ
Sources/Sentry/Profiling/SentryLaunchProfiling.m 90.163% <100.000%> (ø)
...entry/Profiling/SentryProfiledTracerConcurrency.mm 96.759% <100.000%> (ø)
Sources/Sentry/Profiling/SentryTraceProfiler.mm 95.522% <100.000%> (ø)
Sources/Sentry/SentryFileManager.m 95.468% <ø> (ø)
Sources/Sentry/SentryTracer.m 97.158% <100.000%> (ø)
...yProfilerTests/SentryAppLaunchProfilingTests.swift 100.000% <100.000%> (ø)
...ts/SentryTests/Helper/SentryFileManagerTests.swift 96.714% <100.000%> (ø)
Sources/Sentry/PrivateSentrySDKOnly.mm 23.902% <0.000%> (ø)

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 b8d0e7d...8b5cabe. Read the comment docs.

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

@armcknight armcknight force-pushed the armcknight/profiling/new-continuous-apis/7-combine-public-methods branch from 5a35e76 to d1dfcd0 Compare March 21, 2025 08:34
@armcknight armcknight force-pushed the armcknight/profiling/new-continuous-apis/8-function-renames branch from 80190da to b6da0a4 Compare March 21, 2025 08:34
Copy link

github-actions bot commented Mar 21, 2025

🚨 Detected changes in high risk code 🚨

High-risk code can easily blow up and is hard to test. We had severe bugs in the past. Be extra careful when changing these files, and have an extra careful look at these:

  • Sources/Sentry/SentryFileManager.m

Copy link
Member

@philipphofmann philipphofmann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for opening an extra PR 💯. LGTM

…unch-profiling' into armcknight/profiling/new-continuous-apis/7-combine-public-methods
…ic-methods' into armcknight/profiling/new-continuous-apis/8-function-renames
Copy link

github-actions bot commented Mar 21, 2025

🚨 Detected changes in high risk code 🚨

High-risk code can easily blow up and is hard to test. We had severe bugs in the past. Be extra careful when changing these files, and have an extra careful look at these:

  • Sources/Sentry/SentryFileManager.m

…unch-profiling' into armcknight/profiling/new-continuous-apis/7-combine-public-methods
…ic-methods' into armcknight/profiling/new-continuous-apis/8-function-renames
Copy link

github-actions bot commented Mar 31, 2025

🚨 Detected changes in high risk code 🚨

High-risk code can easily blow up and is hard to test. We had severe bugs in the past. Be extra careful when changing these files, and have an extra careful look at these:

  • Sources/Sentry/SentryFileManager.m

…unch-profiling' into armcknight/profiling/new-continuous-apis/7-combine-public-methods
…ic-methods' into armcknight/profiling/new-continuous-apis/8-function-renames
…ne-public-methods' into armcknight/profiling/new-continuous-apis/8-function-renames
…unch-profiling' into armcknight/profiling/new-continuous-apis/7-combine-public-methods
…ic-methods' into armcknight/profiling/new-continuous-apis/8-function-renames
…ic-methods' into armcknight/profiling/new-continuous-apis/8-function-renames
Copy link

github-actions bot commented Mar 31, 2025

🚨 Detected changes in high risk code 🚨

High-risk code can easily blow up and is hard to test. We had severe bugs in the past. Be extra careful when changing these files, and have an extra careful look at these:

  • Sources/Sentry/SentryFileManager.m

Base automatically changed from armcknight/profiling/new-continuous-apis/7-combine-public-methods to armcknight/git-chain/profiling/new-continuous-apis/6-launch-profiling April 1, 2025 00:27
…unch-profiling' into armcknight/profiling/new-continuous-apis/8-function-renames
Copy link

github-actions bot commented Apr 1, 2025

🚨 Detected changes in high risk code 🚨

High-risk code can easily blow up and is hard to test. We had severe bugs in the past. Be extra careful when changing these files, and have an extra careful look at these:

  • Sources/Sentry/SentryFileManager.m

@armcknight armcknight merged commit d21e64c into armcknight/git-chain/profiling/new-continuous-apis/6-launch-profiling Apr 1, 2025
48 of 49 checks passed
@armcknight armcknight deleted the armcknight/profiling/new-continuous-apis/8-function-renames branch April 1, 2025 00:29
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.

3 participants