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): prepare for new public API #3833

Conversation

armcknight
Copy link
Member

  • rename SentryProfiler.h to SentryProfiler+Private.h in case we need a public-facing SentryProfiler.h without all the interface that was already in that file
  • there was already a SentryProfiler+Private.h; split up what was in there between the new one (nee SentryProfiler.h) and SentryProfiler+Test.h, depending on where the declarations are actually needed
  • there were also two SentryProfiler+Test.h files: one in the SDK, and one in the test utils lib; remove the one in the SDK, and move its declarations to the one in the test lib along with the appropriate ones moved from the old SentryProfiler+Private.h that was deleted to make way for renaming SentryProfiler.h to SentryProfiler+Private.h

#skip-changelog

for #3555

…ing new public API

- there was already a SentryProfiler+Private.h; split up what was in there between the new one (nee SentryProfiler.h) and SentryProfiler+Test.h, depending on where the declarations are actually needed
- there were also two SentryProfiler+Test.h files: one in the SDK, and one in the test utils lib; remove the one in the SDK, and move its declarations to the one in the test lib along with the appropriate ones moved from the old SentryProfiler+Private.h that was deleted to make way for renaming SentryProfiler.h to SentryProfiler+Private.h
Base automatically changed from armcknight/feat/3555-continuous-profiling/1-feature-flag to main April 5, 2024 13:25
Copy link

codecov bot commented Apr 5, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 90.957%. Comparing base (c0f08e7) to head (08d9d31).

Additional details and impacted files

Impacted file tree graph

@@              Coverage Diff              @@
##              main     #3833       +/-   ##
=============================================
+ Coverage   90.948%   90.957%   +0.009%     
=============================================
  Files          560       560               
  Lines        44256     44256               
  Branches     15779     15784        +5     
=============================================
+ Hits         40250     40254        +4     
+ Misses        3936      3824      -112     
- Partials        70       178      +108     
Files Coverage Δ
Sources/Sentry/PrivateSentrySDKOnly.mm 87.155% <ø> (ø)
Sources/Sentry/Profiling/SentryLaunchProfiling.m 62.666% <ø> (ø)
Sources/Sentry/SentryFramesTracker.m 100.000% <ø> (ø)
Sources/Sentry/SentryTracer.m 96.897% <ø> (+0.238%) ⬆️
Tests/SentryProfilerTests/SentryProfilerTests.mm 98.773% <ø> (ø)
...ts/SentryTests/Transaction/SentryTracerObjCTests.m 100.000% <ø> (ø)

... and 31 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 c0f08e7...08d9d31. Read the comment docs.

Copy link

github-actions bot commented Apr 5, 2024

Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1233.65 ms 1250.08 ms 16.43 ms
Size 21.58 KiB 573.85 KiB 552.27 KiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
8838e54 1252.06 ms 1258.20 ms 6.14 ms
b15521e 1246.51 ms 1262.96 ms 16.45 ms
89b12eb 1236.02 ms 1246.63 ms 10.61 ms
45d3ca5 1248.27 ms 1255.48 ms 7.21 ms
42ef6ba 1201.67 ms 1214.35 ms 12.67 ms
742d4b6 1230.41 ms 1247.23 ms 16.83 ms
46267d8 1229.80 ms 1244.16 ms 14.37 ms
443723a 1205.24 ms 1220.52 ms 15.28 ms
e90aa6d 1277.38 ms 1303.18 ms 25.81 ms
f25febb 1224.69 ms 1247.38 ms 22.68 ms

App size

Revision Plain With Sentry Diff
8838e54 22.85 KiB 413.45 KiB 390.60 KiB
b15521e 21.58 KiB 573.17 KiB 551.59 KiB
89b12eb 20.76 KiB 432.88 KiB 412.11 KiB
45d3ca5 20.76 KiB 427.54 KiB 406.78 KiB
42ef6ba 21.58 KiB 417.86 KiB 396.28 KiB
742d4b6 21.58 KiB 546.20 KiB 524.62 KiB
46267d8 21.58 KiB 572.21 KiB 550.63 KiB
443723a 20.76 KiB 414.44 KiB 393.68 KiB
e90aa6d 21.58 KiB 418.82 KiB 397.24 KiB
f25febb 21.58 KiB 414.92 KiB 393.34 KiB

Previous results on branch: armcknight/feat/3555-continuous-profiling/2-privatize-existing-api

Startup times

Revision Plain With Sentry Diff
6bb8465 1231.29 ms 1243.78 ms 12.49 ms

App size

Revision Plain With Sentry Diff
6bb8465 21.58 KiB 573.29 KiB 551.71 KiB

Copy link
Contributor

@brustolin brustolin left a comment

Choose a reason for hiding this comment

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

LGTM

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.

LGTM 🚀

…continuous-profiling/2-privatize-existing-api
@armcknight armcknight merged commit 3de9971 into main Apr 10, 2024
70 checks passed
@armcknight armcknight deleted the armcknight/feat/3555-continuous-profiling/2-privatize-existing-api branch April 10, 2024 20:04
dKasabwala pushed a commit to dKasabwala/sentry-cocoa that referenced this pull request May 6, 2024
threema-matteo pushed a commit to threema-ch/sentry-cocoa that referenced this pull request May 21, 2024
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