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: extract implementations of other classes from SentryProfiler.mm #3132

Merged
merged 4 commits into from
Jul 7, 2023

Conversation

armcknight
Copy link
Member

I forgot to move the implementation of SentryProfilerState in #3124

#skip-changelog

@@ -3086,37 +3091,40 @@
8405A517279906EF001B38A1 /* Profiling */ = {
isa = PBXGroup;
children = (
84354E0F29BF944900CDBB8B /* SentryProfileTimeseries.h */,
Copy link
Member Author

Choose a reason for hiding this comment

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

I sorted the contents of the Profiling group by name.

@github-actions
Copy link

github-actions bot commented Jul 7, 2023

Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1229.10 ms 1247.75 ms 18.65 ms
Size 22.84 KiB 401.44 KiB 378.60 KiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
b9b0f0a 1220.20 ms 1229.27 ms 9.06 ms
06548c0 1225.58 ms 1244.70 ms 19.12 ms
f8fc36d 1226.31 ms 1247.80 ms 21.49 ms
adca747 1250.82 ms 1267.57 ms 16.76 ms
6bc5ae5 1227.78 ms 1231.76 ms 3.98 ms
c00eafe 1253.04 ms 1256.41 ms 3.37 ms
f0283e8 1253.36 ms 1263.12 ms 9.76 ms
cf724da 1226.61 ms 1235.70 ms 9.09 ms
f938d24 1223.26 ms 1242.12 ms 18.86 ms
9ef729b 1228.79 ms 1245.36 ms 16.57 ms

App size

Revision Plain With Sentry Diff
b9b0f0a 20.76 KiB 434.94 KiB 414.18 KiB
06548c0 20.76 KiB 427.35 KiB 406.59 KiB
f8fc36d 20.76 KiB 419.70 KiB 398.94 KiB
adca747 20.76 KiB 401.36 KiB 380.60 KiB
6bc5ae5 20.76 KiB 401.39 KiB 380.63 KiB
c00eafe 20.76 KiB 432.88 KiB 412.12 KiB
f0283e8 20.76 KiB 393.36 KiB 372.60 KiB
cf724da 20.76 KiB 430.52 KiB 409.76 KiB
f938d24 20.76 KiB 434.88 KiB 414.12 KiB
9ef729b 20.76 KiB 432.88 KiB 412.12 KiB

Comment on lines -11 to +24
@interface SentryProfilingMutableState : NSObject
@interface SentryProfilerMutableState : NSObject
Copy link
Member Author

@armcknight armcknight Jul 7, 2023

Choose a reason for hiding this comment

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

I'd previously added this to a file called SentryProfilerState but the interface name was SentryProfilingState. Aligning them both to reduce confusion.

@codecov
Copy link

codecov bot commented Jul 7, 2023

Codecov Report

Merging #3132 (f7bfb88) into main (6c31077) will increase coverage by 0.020%.
The diff coverage is 98.473%.

Additional details and impacted files

Impacted file tree graph

@@              Coverage Diff              @@
##              main     #3132       +/-   ##
=============================================
+ Coverage   89.125%   89.146%   +0.020%     
=============================================
  Files          500       501        +1     
  Lines        53963     53910       -53     
  Branches     19321     19274       -47     
=============================================
- Hits         48095     48059       -36     
+ Misses        4900      4884       -16     
+ Partials       968       967        -1     
Impacted Files Coverage Δ
Sources/Sentry/SentryProfileTimeseries.mm 50.000% <ø> (ø)
Sources/Sentry/Profiling/SentryProfilerState.mm 98.387% <98.387%> (ø)
Sources/Sentry/SentryProfiler.mm 77.881% <100.000%> (-5.714%) ⬇️
Tests/SentryProfilerTests/SentryProfilerTests.mm 98.728% <100.000%> (ø)

... and 11 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 6c31077...f7bfb88. Read the comment docs.

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

@armcknight armcknight merged commit 9acdca2 into main Jul 7, 2023
67 checks passed
@armcknight armcknight deleted the armcknight/ref/extract-profiler-state-code branch July 7, 2023 18:59
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