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: compile out more code from macOS builds that only works for UIKit #3157

Merged
merged 6 commits into from
Jul 18, 2023

Conversation

armcknight
Copy link
Member

@armcknight armcknight commented Jul 15, 2023

📜 Description

Compile out more class (and their imports) that are for features that only work with UIKit.

💡 Motivation and Context

Improves type safety, reduces the size of the maOS SDK by 2% (~101 KB) and runs slightly fewer tests for macOS.

#skip-changelog

@codecov
Copy link

codecov bot commented Jul 15, 2023

Codecov Report

Merging #3157 (e0a817c) into main (06bac56) will decrease coverage by 0.037%.
The diff coverage is 98.387%.

❗ Current head e0a817c differs from pull request most recent head 34f8bb4. Consider uploading reports for the commit 34f8bb4 to get more accurate results

Additional details and impacted files

Impacted file tree graph

@@              Coverage Diff              @@
##              main     #3157       +/-   ##
=============================================
- Coverage   89.139%   89.102%   -0.037%     
=============================================
  Files          502       502               
  Lines        53977     53904       -73     
  Branches     19332     19311       -21     
=============================================
- Hits         48115     48030       -85     
- Misses        4894      5019      +125     
+ Partials       968       855      -113     
Impacted Files Coverage Δ
Sources/Sentry/SentryAppStartMeasurement.m 57.142% <ø> (ø)
Sources/Sentry/SentryAppStartTrackingIntegration.m 100.000% <ø> (ø)
Sources/Sentry/SentryDependencyContainer.m 94.392% <ø> (ø)
Sources/Sentry/SentryFramesTrackingIntegration.m 100.000% <ø> (ø)
Sources/Sentry/SentryHub.m 98.454% <ø> (ø)
...rces/Sentry/SentryPerformanceTrackingIntegration.m 100.000% <ø> (ø)
Sources/Sentry/SentryScreenshot.m 76.315% <ø> (ø)
Sources/Sentry/SentryScreenshotIntegration.m 90.000% <ø> (ø)
Sources/Sentry/SentryTimeToDisplayTracker.m 100.000% <ø> (ø)
Sources/Sentry/SentryUIApplication.m 88.571% <ø> (ø)
... and 34 more

... 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 06bac56...34f8bb4. Read the comment docs.

@github-actions
Copy link

github-actions bot commented Jul 15, 2023

Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1217.82 ms 1235.08 ms 17.26 ms
Size 22.84 KiB 403.18 KiB 380.34 KiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
fc163f5 1198.05 ms 1227.76 ms 29.71 ms
1bbcb9c 1192.51 ms 1231.96 ms 39.45 ms
c0b4b71 1246.98 ms 1256.77 ms 9.79 ms
90d17d3 1261.18 ms 1278.18 ms 17.00 ms
eed479f 1198.93 ms 1228.12 ms 29.19 ms
ff09c7e 1244.86 ms 1246.68 ms 1.82 ms
154f795 1250.38 ms 1274.54 ms 24.16 ms
369222e 1232.14 ms 1258.90 ms 26.76 ms
596ccc1 1221.57 ms 1236.82 ms 15.25 ms
591a01b 1197.94 ms 1222.53 ms 24.59 ms

App size

Revision Plain With Sentry Diff
fc163f5 20.76 KiB 436.30 KiB 415.54 KiB
1bbcb9c 20.76 KiB 426.10 KiB 405.34 KiB
c0b4b71 20.76 KiB 430.98 KiB 410.22 KiB
90d17d3 20.76 KiB 432.17 KiB 411.41 KiB
eed479f 20.76 KiB 433.18 KiB 412.42 KiB
ff09c7e 20.76 KiB 427.77 KiB 407.00 KiB
154f795 20.76 KiB 435.25 KiB 414.49 KiB
369222e 20.76 KiB 419.67 KiB 398.91 KiB
596ccc1 22.84 KiB 401.44 KiB 378.60 KiB
591a01b 22.84 KiB 401.67 KiB 378.83 KiB

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

Startup times

Revision Plain With Sentry Diff
e929c27 1244.80 ms 1253.02 ms 8.22 ms
fe98a1e 1205.94 ms 1226.60 ms 20.66 ms

App size

Revision Plain With Sentry Diff
e929c27 22.84 KiB 403.21 KiB 380.37 KiB
fe98a1e 22.84 KiB 403.18 KiB 380.34 KiB

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. As this reduces the SDK size for macOS by 2% we could add an entry to the changelog under an improvement section.

@armcknight armcknight force-pushed the armcknight/ref/compile-out-more-SENTRY_HAS_UIKIT branch from 9bdceb3 to e0a817c Compare July 18, 2023 06:35
@armcknight
Copy link
Member Author

LGTM. As this reduces the SDK size for macOS by 2% we could add an entry to the changelog under an improvement section.

True that! Gotta take the opportunity to brag whenever possible :)

@armcknight armcknight merged commit 27cd119 into main Jul 18, 2023
@armcknight armcknight deleted the armcknight/ref/compile-out-more-SENTRY_HAS_UIKIT branch July 18, 2023 18:32
vaind added a commit to getsentry/sentry-dart that referenced this pull request Aug 17, 2023
Broken by getsentry/sentry-cocoa#3157 (v8.9.2) because the `SentryAppStartMeasurement` is not exported anymore.
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.

2 participants