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: gather metric samples at profile start/end #3272

Merged
merged 5 commits into from Sep 18, 2023

Conversation

armcknight
Copy link
Member

📜 Description

Gather a metric sample right at the beginning and end of a profile.

💡 Motivation and Context

We previously gathered metrics samples at 100 Hz, but not until the first 100 ms of a profile had elapsed, and there could've been up to 99ms without a sample at the end of a profile.

💚 How did you test it?

Unit test update.

@github-actions
Copy link

github-actions bot commented Sep 8, 2023

Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1253.38 ms 1267.86 ms 14.48 ms
Size 22.85 KiB 407.87 KiB 385.02 KiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
be2977c 1202.51 ms 1221.32 ms 18.81 ms
c00eafe 1253.04 ms 1256.41 ms 3.37 ms
b9d59f7 1250.71 ms 1257.78 ms 7.06 ms
e1eed6b 1224.63 ms 1234.84 ms 10.20 ms
bd2afa6 1241.37 ms 1246.20 ms 4.83 ms
ea73af6 1230.96 ms 1244.98 ms 14.02 ms
d760c3f 1228.58 ms 1246.22 ms 17.64 ms
f4e0299 1230.33 ms 1249.68 ms 19.35 ms
c50d363 1215.10 ms 1231.82 ms 16.72 ms
0d32275 1245.80 ms 1267.80 ms 22.00 ms

App size

Revision Plain With Sentry Diff
be2977c 22.85 KiB 407.67 KiB 384.83 KiB
c00eafe 20.76 KiB 432.88 KiB 412.12 KiB
b9d59f7 22.85 KiB 405.77 KiB 382.93 KiB
e1eed6b 20.76 KiB 432.17 KiB 411.41 KiB
bd2afa6 20.76 KiB 420.55 KiB 399.79 KiB
ea73af6 20.76 KiB 425.75 KiB 404.99 KiB
d760c3f 22.84 KiB 403.17 KiB 380.33 KiB
f4e0299 20.76 KiB 427.54 KiB 406.78 KiB
c50d363 20.76 KiB 432.68 KiB 411.92 KiB
0d32275 22.84 KiB 403.14 KiB 380.29 KiB

Previous results on branch: armcknight/fix/profiling-metric-collection-start-end

Startup times

Revision Plain With Sentry Diff
b7841c0 1215.61 ms 1235.82 ms 20.21 ms
dcc5dc6 1247.71 ms 1258.12 ms 10.41 ms

App size

Revision Plain With Sentry Diff
b7841c0 22.85 KiB 407.75 KiB 384.90 KiB
dcc5dc6 22.85 KiB 407.82 KiB 384.97 KiB

@codecov
Copy link

codecov bot commented Sep 9, 2023

Codecov Report

Merging #3272 (359e03a) into main (881a955) will decrease coverage by 0.028%.
The diff coverage is 95.348%.

Impacted file tree graph

@@              Coverage Diff              @@
##              main     #3272       +/-   ##
=============================================
- Coverage   89.223%   89.195%   -0.028%     
=============================================
  Files          501       502        +1     
  Lines        53949     54273      +324     
  Branches     19170     19485      +315     
=============================================
+ Hits         48135     48409      +274     
- Misses        4958      4999       +41     
- Partials       856       865        +9     
Files Changed Coverage
Sources/Sentry/SentryProfiler.mm 75.000%
Sources/Sentry/SentryDispatchSourceWrapper.m 100.000%
Sources/Sentry/SentryMetricProfiler.mm 100.000%
Sources/Sentry/SentryTracer.m 100.000%
...SentryProfilerTests/SentryProfilerSwiftTests.swift 100.000%

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 881a955...359e03a. 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

@armcknight armcknight merged commit cb8ed46 into main Sep 18, 2023
19 of 20 checks passed
@armcknight armcknight deleted the armcknight/fix/profiling-metric-collection-start-end branch September 18, 2023 18:35
@github-actions
Copy link

Fails
🚫 Please consider adding a changelog entry for the next release.

Instructions and example for changelog

Please add an entry to CHANGELOG.md to the "Unreleased" section. Make sure the entry includes this PR's number.

Example:

## Unreleased

- gather metric samples at profile start/end ([#3272](https://github.com/getsentry/sentry-cocoa/pull/3272))

If none of the above apply, you can opt out of this check by adding #skip-changelog to the PR description.

Generated by 🚫 dangerJS against d81067c

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