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.

Sign up for GitHub

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

Jump to bottom

enhancement(frame-tracking): only store slow and frozen frames for calculation #2337

Merged
merged 10 commits into from
Oct 10, 2024
Merged

enhancement(frame-tracking): only store slow and frozen frames for calculation #2337

merged 10 commits into from
Oct 10, 2024

Conversation

buenaflor
Copy link
Contributor

@buenaflor buenaflor commented Oct 8, 2024 •
edited
Loading

📜 Description

Reduce the number of frames being stored at once, we don't need to keep track of frames that are within expected duration, only the slow and frozen ones

💡 Motivation and Context

Improve performance of calculation and memory footprint

💚 How did you test it?

Existing tests assert correctness of calculations

📝 Checklist

  • I reviewed submitted code
  • I added tests to verify changes
  • No new PII added or SDK only sends newly added PII if sendDefaultPii is enabled
  • I updated the docs if needed
  • All tests passing
  • No breaking changes

🔮 Next steps

@buenaflor buenaflor changed the title enhancement: only store slow and frozen frames for calculation enhancement(frame-tracking): only store slow and frozen frames for calculation Oct 8, 2024
@codecov Codecov
Copy link

codecov bot commented Oct 8, 2024 •
edited
Loading

Codecov Report

Attention: Patch coverage is 78.72340% with 10 lines in your changes missing coverage. Please review.

Project coverage is 90.57%. Comparing base (73cf0b4) to head (30f0031).
Report is 3 commits behind head on main.

Files with missing lines Patch % Lines
flutter/lib/src/span_frame_metrics_collector.dart 78.72% 10 Missing ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##             main    #2337      +/-   ##
==========================================
+ Coverage   86.38%   90.57%   +4.19%     
==========================================
  Files         247       73     -174     
  Lines        8615     2409    -6206     
==========================================
- Hits         7442     2182    -5260     
+ Misses       1173      227     -946

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Oct 8, 2024 •
edited
Loading

Android Performance metrics 🚀

  Plain With Sentry Diff
Startup time 470.89 ms 530.62 ms 59.73 ms
Size 6.49 MiB 7.56 MiB 1.07 MiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
8ced2dc 295.58 ms 336.49 ms 40.91 ms
559d28f 302.35 ms 339.53 ms 37.18 ms
62ae1b3 419.60 ms 494.14 ms 74.54 ms
6d7a391 331.94 ms 367.04 ms 35.10 ms
3ad66e4 406.90 ms 481.48 ms 74.58 ms
3334ac1 303.98 ms 366.65 ms 62.67 ms
0118295 365.71 ms 438.56 ms 72.85 ms
7e7f0b1 387.44 ms 459.14 ms 71.70 ms
fe4aa56 356.06 ms 428.67 ms 72.61 ms
25161f4 353.98 ms 431.94 ms 77.96 ms

App size

Revision Plain With Sentry Diff
8ced2dc 6.06 MiB 7.03 MiB 990.29 KiB
559d28f 5.94 MiB 6.92 MiB 1001.70 KiB
62ae1b3 6.35 MiB 7.40 MiB 1.05 MiB
6d7a391 5.94 MiB 6.95 MiB 1.01 MiB
3ad66e4 6.33 MiB 7.26 MiB 947.04 KiB
3334ac1 6.06 MiB 7.03 MiB 993.54 KiB
0118295 6.33 MiB 7.26 MiB 947.07 KiB
7e7f0b1 6.35 MiB 7.34 MiB 1008.33 KiB
fe4aa56 6.06 MiB 7.10 MiB 1.04 MiB
25161f4 6.27 MiB 7.20 MiB 960.44 KiB

Previous results on branch: enh/dont-cache-normal-frames

Startup times

Revision Plain With Sentry Diff
6f542c3 496.92 ms 538.02 ms 41.10 ms
eb6c300 438.89 ms 509.16 ms 70.27 ms
9fc3946 491.21 ms 562.57 ms 71.36 ms
b7d8bda 592.26 ms 625.72 ms 33.47 ms

App size

Revision Plain With Sentry Diff
6f542c3 6.49 MiB 7.56 MiB 1.07 MiB
eb6c300 6.49 MiB 7.56 MiB 1.07 MiB
9fc3946 6.49 MiB 7.56 MiB 1.07 MiB
b7d8bda 6.49 MiB 7.56 MiB 1.07 MiB

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Oct 8, 2024 •
edited
Loading

iOS Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1223.92 ms 1244.12 ms 20.21 ms
Size 8.38 MiB 9.74 MiB 1.36 MiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
24b6e60 1250.69 ms 1268.63 ms 17.94 ms
2e1e4ae 1254.41 ms 1278.55 ms 24.14 ms
6a5a65d 1237.22 ms 1250.29 ms 13.07 ms
73d70bf 1249.08 ms 1268.41 ms 19.33 ms
7faee57 1232.65 ms 1246.10 ms 13.45 ms
43760f9 1216.07 ms 1238.51 ms 22.44 ms
6daa837 1250.42 ms 1265.60 ms 15.18 ms
d4120ac 1260.61 ms 1274.09 ms 13.47 ms
d883d62 1221.39 ms 1230.18 ms 8.80 ms
43abc4f 1248.33 ms 1272.86 ms 24.52 ms

App size

Revision Plain With Sentry Diff
24b6e60 8.32 MiB 9.38 MiB 1.06 MiB
2e1e4ae 8.34 MiB 9.65 MiB 1.31 MiB
6a5a65d 8.34 MiB 9.65 MiB 1.31 MiB
73d70bf 8.38 MiB 9.70 MiB 1.33 MiB
7faee57 8.33 MiB 9.64 MiB 1.31 MiB
43760f9 8.29 MiB 9.36 MiB 1.07 MiB
6daa837 8.33 MiB 9.40 MiB 1.07 MiB
d4120ac 8.28 MiB 9.34 MiB 1.06 MiB
d883d62 8.29 MiB 9.36 MiB 1.07 MiB
43abc4f 8.33 MiB 9.54 MiB 1.21 MiB

Previous results on branch: enh/dont-cache-normal-frames

Startup times

Revision Plain With Sentry Diff
9fc3946 1254.90 ms 1278.96 ms 24.06 ms
6f542c3 1248.27 ms 1278.76 ms 30.49 ms
eb6c300 1241.42 ms 1270.02 ms 28.60 ms
b7d8bda 1252.67 ms 1273.08 ms 20.41 ms

App size

Revision Plain With Sentry Diff
9fc3946 8.38 MiB 9.74 MiB 1.36 MiB
6f542c3 8.38 MiB 9.74 MiB 1.36 MiB
eb6c300 8.38 MiB 9.74 MiB 1.36 MiB
b7d8bda 8.38 MiB 9.74 MiB 1.36 MiB

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Oct 9, 2024 •
edited by buenaflor
Loading

🚨 Detected changes in high risk code 🚨

High-risk code has higher potential to break the SDK and may be hard to test. To prevent severe bugs, apply the rollout process for releasing such changes and be extra careful when changing and reviewing these files:

  • flutter/lib/src/span_frame_metrics_collector.dart

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Oct 9, 2024 •
edited by buenaflor
Loading

🚨 Detected changes in high risk code 🚨

High-risk code has higher potential to break the SDK and may be hard to test. To prevent severe bugs, apply the rollout process for releasing such changes and be extra careful when changing and reviewing these files:

  • flutter/lib/src/span_frame_metrics_collector.dart

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Oct 9, 2024 •
edited by buenaflor
Loading

🚨 Detected changes in high risk code 🚨

High-risk code has higher potential to break the SDK and may be hard to test. To prevent severe bugs, apply the rollout process for releasing such changes and be extra careful when changing and reviewing these files:

  • flutter/lib/src/span_frame_metrics_collector.dart

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Oct 9, 2024 •
edited by buenaflor
Loading

🚨 Detected changes in high risk code 🚨

High-risk code has higher potential to break the SDK and may be hard to test. To prevent severe bugs, apply the rollout process for releasing such changes and be extra careful when changing and reviewing these files:

  • flutter/lib/src/span_frame_metrics_collector.dart

stefanosiano
stefanosiano approved these changes Oct 10, 2024
View reviewed changes
Copy link
Member

@stefanosiano stefanosiano left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice!

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Oct 10, 2024 •
edited by buenaflor
Loading

🚨 Detected changes in high risk code 🚨

High-risk code has higher potential to break the SDK and may be hard to test. To prevent severe bugs, apply the rollout process for releasing such changes and be extra careful when changing and reviewing these files:

  • flutter/lib/src/span_frame_metrics_collector.dart

@buenaflor buenaflor merged commit 6d50a9b into main Oct 10, 2024
53 checks passed
@buenaflor buenaflor deleted the enh/dont-cache-normal-frames branch October 10, 2024 11:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@stefanosiano stefanosiano stefanosiano approved these changes

@krystofwoldrich krystofwoldrich Awaiting requested review from krystofwoldrich krystofwoldrich is a code owner

@martinhaintz martinhaintz Awaiting requested review from martinhaintz

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@buenaflor @stefanosiano