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

fix(launch profiling): debug logs and cleanup #3635

Conversation

armcknight
Copy link
Member

@armcknight armcknight commented Feb 14, 2024

Follows #3634 and followed by #3636

Remove some global variables that are no longer needed now that we use a dedicated transaction for app launch.

Remove some old API from the backup config file strategy we no longer use. It was causing some build failures in tests.

Also some debug logs that were helpful to me.

#skip-changelog

Copy link

github-actions bot commented Feb 14, 2024

Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1229.15 ms 1251.14 ms 21.99 ms
Size 21.58 KiB 422.62 KiB 401.04 KiB

Previous results on branch: armcknight/feat/launch-profiling5-cleanup

Startup times

Revision Plain With Sentry Diff
4916322 1211.00 ms 1220.00 ms 9.00 ms
b4dffe4 1205.65 ms 1222.49 ms 16.84 ms

App size

Revision Plain With Sentry Diff
4916322 21.58 KiB 422.53 KiB 400.95 KiB
b4dffe4 21.58 KiB 422.53 KiB 400.95 KiB

@armcknight
Copy link
Member Author

UI test TopViewControllerTests.testPagesViewController() failed in asan, reran it

@armcknight
Copy link
Member Author

Two unit tests failing:

  • SentryCrashInstallationReporterTests.testReportIsSentAndDeleted()
  • -[SentryCrashReportStore_Tests testPruneReports]
    rerunning those

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.

Looks good.

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

Copy link

codecov bot commented Feb 15, 2024

Codecov Report

Attention: 1 lines in your changes are missing coverage. Please review.

Comparison is base (775818a) 89.189% compared to head (54c5990) 89.167%.

Additional details and impacted files

Impacted file tree graph

@@                                   Coverage Diff                                    @@
##           armcknight/feat/launch-profiling4-end-to-end-check     #3635       +/-   ##
========================================================================================
- Coverage                                              89.189%   89.167%   -0.022%     
========================================================================================
  Files                                                     532       532               
  Lines                                                   58672     58659       -13     
  Branches                                                21044     21046        +2     
========================================================================================
- Hits                                                    52329     52305       -24     
- Misses                                                   5303      5309        +6     
- Partials                                                 1040      1045        +5     
Files Coverage Δ
Sources/Sentry/SentryProfiler.mm 78.205% <100.000%> (+1.282%) ⬆️
Sources/Sentry/SentryTracer.m 96.309% <ø> (+0.664%) ⬆️
Sources/Sentry/Profiling/SentryLaunchProfiling.m 62.962% <0.000%> (+2.248%) ⬆️

... 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 775818a...54c5990. Read the comment docs.

@armcknight armcknight merged commit aad4df2 into armcknight/feat/launch-profiling4-end-to-end-check Feb 15, 2024
46 checks passed
@armcknight armcknight deleted the armcknight/feat/launch-profiling5-cleanup branch February 15, 2024 08:33
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