Skip to content

ref: compile out more code from platforms that don't support profiling #3159

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

Conversation

armcknight
Copy link
Member

@armcknight armcknight commented Jul 15, 2023

Like #3157 and #3158. Improves type safety, reduces the size of the tvOS SDK and runs slightly fewer tests for tvOS.

#skip-changelog

Verified

This commit was signed with the committer’s verified signature.
armcknight Andrew McKnight

Verified

This commit was signed with the committer’s verified signature.
armcknight Andrew McKnight
…or UIKit

Verified

This commit was signed with the committer’s verified signature.
armcknight Andrew McKnight
…or UIKit

Verified

This commit was signed with the committer’s verified signature.
armcknight Andrew McKnight
…or UIKit

Verified

This commit was signed with the committer’s verified signature.
armcknight Andrew McKnight

Verified

This commit was signed with the committer’s verified signature.
armcknight Andrew McKnight
…RIC_KIT

Verified

This commit was signed with the committer’s verified signature.
armcknight Andrew McKnight
…HAS_METRIC_KIT

Verified

This commit was signed with the committer’s verified signature.
armcknight Andrew McKnight
@codecov
Copy link

codecov bot commented Jul 15, 2023

Codecov Report

Merging #3159 (3ed09da) into main (313b5fa) will decrease coverage by 0.006%.
The diff coverage is 100.000%.

Additional details and impacted files

Impacted file tree graph

@@              Coverage Diff              @@
##              main     #3159       +/-   ##
=============================================
- Coverage   89.111%   89.106%   -0.006%     
=============================================
  Files          502       502               
  Lines        53919     53865       -54     
  Branches     19337     19307       -30     
=============================================
- Hits         48048     47997       -51     
+ Misses        5015      4903      -112     
- Partials       856       965      +109     
Impacted Files Coverage Δ
Sources/Sentry/SentryOptions.m 97.149% <ø> (ø)
Sources/Sentry/SentryProfilesSampler.m 94.117% <ø> (-0.222%) ⬇️
Sources/Sentry/SentryTracer.m 96.500% <ø> (-0.185%) ⬇️
Tests/SentryTests/SentrySDKTests.swift 97.373% <ø> (ø)
...ts/SentryTests/Transaction/SentryTracerObjCTests.m 100.000% <ø> (ø)
Sources/Sentry/SentryHub.m 98.464% <100.000%> (+0.006%) ⬆️

... and 29 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 313b5fa...3ed09da. Read the comment docs.

@github-actions
Copy link
Contributor

github-actions bot commented Jul 15, 2023

Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1199.66 ms 1232.54 ms 32.88 ms
Size 22.84 KiB 402.56 KiB 379.72 KiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
7192d9e 1221.86 ms 1248.28 ms 26.42 ms
31ac438 1216.17 ms 1242.68 ms 26.51 ms
7c37d8e 1256.00 ms 1259.36 ms 3.36 ms
eaf3840 1264.94 ms 1274.04 ms 9.10 ms
794f87f 1225.78 ms 1243.46 ms 17.68 ms
7fb7afb 1235.00 ms 1256.81 ms 21.81 ms
4d68229 1233.50 ms 1262.92 ms 29.42 ms
2af280d 1232.31 ms 1249.98 ms 17.67 ms
33b2f51 1251.24 ms 1259.18 ms 7.94 ms
98752f3 1240.61 ms 1259.80 ms 19.18 ms

App size

Revision Plain With Sentry Diff
7192d9e 20.76 KiB 431.71 KiB 410.95 KiB
31ac438 20.76 KiB 393.37 KiB 372.61 KiB
7c37d8e 20.76 KiB 426.86 KiB 406.09 KiB
eaf3840 22.84 KiB 401.67 KiB 378.83 KiB
794f87f 20.76 KiB 401.37 KiB 380.61 KiB
7fb7afb 20.76 KiB 419.69 KiB 398.94 KiB
4d68229 20.76 KiB 432.34 KiB 411.58 KiB
2af280d 20.76 KiB 435.22 KiB 414.46 KiB
33b2f51 20.76 KiB 432.17 KiB 411.41 KiB
98752f3 20.76 KiB 435.09 KiB 414.33 KiB

Previous results on branch: armcknight/ref/compile-out-more-SENTRY_TARGET_PROFILING_SUPPORTED

Startup times

Revision Plain With Sentry Diff
091196c 1221.41 ms 1252.17 ms 30.76 ms

App size

Revision Plain With Sentry Diff
091196c 22.84 KiB 403.20 KiB 380.36 KiB

Verified

This commit was signed with the committer’s verified signature.
armcknight Andrew McKnight
… build with import style
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, but CI is complaining.

Base automatically changed from armcknight/ref/compile-out-more-SENTRY_HAS_METRICKIT to main July 18, 2023 20:56

Verified

This commit was signed with the committer’s verified signature.
armcknight Andrew McKnight
…T_PROFILING_SUPPORTED

Verified

This commit was signed with the committer’s verified signature.
armcknight Andrew McKnight

Verified

This commit was signed with the committer’s verified signature.
armcknight Andrew McKnight

Verified

This commit was signed with the committer’s verified signature.
armcknight Andrew McKnight
@armcknight armcknight merged commit 84fb4d9 into main Jul 18, 2023
@armcknight armcknight deleted the armcknight/ref/compile-out-more-SENTRY_TARGET_PROFILING_SUPPORTED branch July 18, 2023 22:49
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.

None yet

2 participants