search

getsentry / sentry-cocoa

The official Sentry SDK for iOS, tvOS, macOS, watchOS.
https://sentry.io/for/cocoa/
MIT License
773 stars 306 forks source link

fix(profiling): put back the missing insertion of initial frame rate #3987

Closed armcknight closed 2 weeks ago

armcknight commented 2 weeks ago

In #3932, this line of code got accidentally removed. I'm not sure if this codepath is ever actually taken. It's not currently covered in tests; to be honest the mocking around GPU timestamps is a mess, and I haven't been able to untangle it to get this particular codepath covered. I hope to make more progress on that in #3985.

But I would like to get this merged before any release goes out, in case something would actually break.

skip-changelog, for #3555

codecov[bot] commented 2 weeks ago

Codecov Report

Attention: Patch coverage is 0% with 1 lines in your changes are missing coverage. Please review.

Project coverage is 90.884%. Comparing base (d914696) to head (2127882).

Additional details and impacted files [![Impacted file tree graph](https://app.codecov.io/gh/getsentry/sentry-cocoa/pull/3987/graphs/tree.svg?width=650&height=150&src=pr&token=PTZKtOJlrs&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=getsentry)](https://app.codecov.io/gh/getsentry/sentry-cocoa/pull/3987?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=getsentry) ```diff @@ Coverage Diff @@ ## main #3987 +/- ## ============================================= + Coverage 90.802% 90.884% +0.081% ============================================= Files 594 594 Lines 46351 46351 Branches 16619 16617 -2 ============================================= + Hits 42088 42126 +38 + Misses 4083 4044 -39 - Partials 180 181 +1 ``` | [Files](https://app.codecov.io/gh/getsentry/sentry-cocoa/pull/3987?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=getsentry) | Coverage Δ | | |---|---|---| | [Sources/Sentry/SentryProfileTimeseries.mm](https://app.codecov.io/gh/getsentry/sentry-cocoa/pull/3987?src=pr&el=tree&filepath=Sources%2FSentry%2FSentryProfileTimeseries.mm&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=getsentry#diff-U291cmNlcy9TZW50cnkvU2VudHJ5UHJvZmlsZVRpbWVzZXJpZXMubW0=) | `53.535% <0.000%> (-0.547%)` | :arrow_down: | ... and [11 files with indirect coverage changes](https://app.codecov.io/gh/getsentry/sentry-cocoa/pull/3987/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=getsentry) ------ [Continue to review full report in Codecov by Sentry](https://app.codecov.io/gh/getsentry/sentry-cocoa/pull/3987?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=getsentry). > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=getsentry) > `Δ = absolute (impact)`, `ø = not affected`, `? = missing data` > Powered by [Codecov](https://app.codecov.io/gh/getsentry/sentry-cocoa/pull/3987?dropdown=coverage&src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=getsentry). Last update [d914696...2127882](https://app.codecov.io/gh/getsentry/sentry-cocoa/pull/3987?dropdown=coverage&src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=getsentry). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=getsentry).
github-actions[bot] commented 2 weeks ago

Performance metrics :rocket:

  Plain With Sentry Diff
Startup time 1220.96 ms 1235.23 ms 14.27 ms
Size 21.58 KiB 629.85 KiB 608.27 KiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
dd0557fb0fec40f5527e4f40ead6f82e092fde0f 1242.04 ms 1250.86 ms 8.82 ms
742d4b693cfd3cf2b7b1d62930d16daaf505d367 1217.04 ms 1229.38 ms 12.33 ms
313b1d969cac3950fa497413ae2d1b5b9e76cada 1240.18 ms 1258.44 ms 18.26 ms
533859f7bae4a14f6c535a2ec7aef4f526f01e83 1237.78 ms 1249.76 ms 11.98 ms
67460f42bd3e7efc8d06f995d22a94a61f657385 1244.56 ms 1255.96 ms 11.40 ms
f0ce81c3105c33add6dada975cec8a32e604b9e5 1231.06 ms 1244.79 ms 13.73 ms
d3abae0e1782d1d9e495befcfa1326f25ca64604 1200.36 ms 1224.22 ms 23.87 ms
56ec5d04c9b2d9c989dfc5e55f904c537bdc2956 1236.65 ms 1261.90 ms 25.25 ms
e32423038e99f0c5d868744421d664cdd55c7383 1254.92 ms 1262.92 ms 8.00 ms
324dc7bbae1d5746f8c5d27e0b4399c560ba2718 1210.33 ms 1244.92 ms 34.59 ms

App size

Revision Plain With Sentry Diff
dd0557fb0fec40f5527e4f40ead6f82e092fde0f 22.85 KiB 411.75 KiB 388.90 KiB
742d4b693cfd3cf2b7b1d62930d16daaf505d367 21.58 KiB 546.19 KiB 524.61 KiB
313b1d969cac3950fa497413ae2d1b5b9e76cada 22.85 KiB 414.11 KiB 391.26 KiB
533859f7bae4a14f6c535a2ec7aef4f526f01e83 22.85 KiB 408.84 KiB 386.00 KiB
67460f42bd3e7efc8d06f995d22a94a61f657385 20.76 KiB 426.15 KiB 405.39 KiB
f0ce81c3105c33add6dada975cec8a32e604b9e5 21.58 KiB 418.33 KiB 396.75 KiB
d3abae0e1782d1d9e495befcfa1326f25ca64604 20.76 KiB 434.92 KiB 414.16 KiB
56ec5d04c9b2d9c989dfc5e55f904c537bdc2956 20.76 KiB 414.44 KiB 393.69 KiB
e32423038e99f0c5d868744421d664cdd55c7383 22.85 KiB 408.88 KiB 386.03 KiB
324dc7bbae1d5746f8c5d27e0b4399c560ba2718 22.85 KiB 408.84 KiB 385.99 KiB