Skip to content

feat: use addTimingsCallback instead of addPostFrameCallback to determine app start end#2405

Merged
buenaflor merged 36 commits into
mainfrom
enh/app-start-time
Nov 20, 2024
Merged

feat: use addTimingsCallback instead of addPostFrameCallback to determine app start end#2405
buenaflor merged 36 commits into
mainfrom
enh/app-start-time

Conversation

@buenaflor

@buenaflor buenaflor commented Nov 12, 2024 •
edited
Loading

Copy link
Copy Markdown
Contributor

📜 Description

Improve app start measurements

💡 Motivation and Context

The addTimingsCallback contains the accurate data for the first frame unlike addPostFramecallback which is not correct as it depends on when SentryFlutter.init is called

addTimingsCallback is always reported immediately for the first frame so it's very suitable to use here, see https://api.flutter.dev/flutter/scheduler/SchedulerBinding/addTimingsCallback.html

💚 How did you test it?

Unit tests, manual tests

📝 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

@github-actions

github-actions Bot commented Nov 12, 2024 •
edited by buenaflor
Loading

Copy link
Copy Markdown
Contributor

🚨 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/integrations/native_app_start_integration.dart

@github-actions

github-actions Bot commented Nov 12, 2024 •
edited
Loading

Copy link
Copy Markdown
Contributor
Messages
📖 Do not forget to update Sentry-docs with your feature once the pull request gets approved.

Generated by 🚫 dangerJS against f993041

@codecov

codecov Bot commented Nov 12, 2024 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

Attention: Patch coverage is 92.30769% with 1 line in your changes missing coverage. Please review.

Project coverage is 86.05%. Comparing base (404f5a9) to head (f993041).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
flutter/lib/src/frame_callback_handler.dart 50.00% 1 Missing ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##             main    #2405      +/-   ##
==========================================
+ Coverage   85.04%   86.05%   +1.00%     
==========================================
  Files         257       81     -176     
  Lines        9194     2875    -6319     
==========================================
- Hits         7819     2474    -5345     
+ Misses       1375      401     -974

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

🚨 Try these New Features:

buenaflor
buenaflor commented Nov 12, 2024
View reviewed changes
Comment thread flutter/lib/src/integrations/native_app_start_integration.dart
Comment thread flutter/test/fake_frame_callback_handler.dart
@github-actions

github-actions Bot commented Nov 12, 2024 •
edited
Loading

Copy link
Copy Markdown
Contributor

Android Performance metrics 🚀

  Plain With Sentry Diff
Startup time 459.28 ms 540.31 ms 81.02 ms
Size 6.49 MiB 7.56 MiB 1.07 MiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
453e1bc 320.41 ms 372.73 ms 52.32 ms
061fed2 434.11 ms 506.49 ms 72.38 ms
9928a74 375.26 ms 456.30 ms 81.04 ms
11fb408 320.10 ms 380.24 ms 60.14 ms
dd76eef 461.37 ms 540.55 ms 79.18 ms
2261c15 370.00 ms 455.88 ms 85.88 ms
d089990 361.67 ms 442.50 ms 80.83 ms
ca7f531 395.69 ms 497.82 ms 102.12 ms
e5b744f 302.70 ms 342.17 ms 39.47 ms
1e094d3 316.22 ms 378.69 ms 62.47 ms

App size

Revision Plain With Sentry Diff
453e1bc 5.94 MiB 6.95 MiB 1.01 MiB
061fed2 6.52 MiB 7.59 MiB 1.06 MiB
9928a74 5.94 MiB 6.96 MiB 1.02 MiB
11fb408 6.06 MiB 7.10 MiB 1.04 MiB
dd76eef 6.35 MiB 7.40 MiB 1.05 MiB
2261c15 6.27 MiB 7.20 MiB 957.75 KiB
d089990 6.34 MiB 7.28 MiB 967.79 KiB
ca7f531 6.33 MiB 7.26 MiB 949.75 KiB
e5b744f 6.06 MiB 7.09 MiB 1.03 MiB
1e094d3 6.16 MiB 7.14 MiB 1004.21 KiB

Previous results on branch: enh/app-start-time

Startup times

Revision Plain With Sentry Diff
21acda1 450.35 ms 492.31 ms 41.96 ms
92b5b84 453.60 ms 485.92 ms 32.32 ms
5990562 463.09 ms 508.14 ms 45.05 ms
ca24b53 475.92 ms 551.68 ms 75.76 ms
b1e9b9c 469.20 ms 500.98 ms 31.78 ms
803684b 455.88 ms 497.16 ms 41.29 ms
d724aac 482.20 ms 512.35 ms 30.14 ms
4def0ce 465.86 ms 494.74 ms 28.88 ms
9be4829 438.42 ms 458.45 ms 20.03 ms
88bc8a5 482.94 ms 515.82 ms 32.88 ms

App size

Revision Plain With Sentry Diff
21acda1 6.49 MiB 7.56 MiB 1.07 MiB
92b5b84 6.49 MiB 7.56 MiB 1.07 MiB
5990562 6.49 MiB 7.56 MiB 1.07 MiB
ca24b53 6.49 MiB 7.56 MiB 1.07 MiB
b1e9b9c 6.49 MiB 7.56 MiB 1.07 MiB
803684b 6.49 MiB 7.56 MiB 1.07 MiB
d724aac 6.49 MiB 7.56 MiB 1.07 MiB
4def0ce 6.49 MiB 7.56 MiB 1.07 MiB
9be4829 6.49 MiB 7.56 MiB 1.07 MiB
88bc8a5 6.49 MiB 7.56 MiB 1.07 MiB

@getsentry getsentry deleted a comment from github-actions Bot Nov 12, 2024
@getsentry getsentry deleted a comment from github-actions Bot Nov 12, 2024
@getsentry getsentry deleted a comment from github-actions Bot Nov 12, 2024
@getsentry getsentry deleted a comment from github-actions Bot Nov 12, 2024
@getsentry getsentry deleted a comment from github-actions Bot Nov 12, 2024
@getsentry getsentry deleted a comment from github-actions Bot Nov 12, 2024
@buenaflor

buenaflor commented Nov 12, 2024

Copy link
Copy Markdown
Contributor Author

cc @stefanosiano @denrase please take a look

@getsentry getsentry deleted a comment from github-actions Bot Nov 12, 2024
denrase
denrase approved these changes Nov 18, 2024
View reviewed changes

@denrase denrase left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Looking good. Only had 2 comment, but those cases work fine when i tested in the example app. 👍

Comment thread flutter/lib/src/integrations/native_app_start_integration.dart
Comment thread flutter/lib/src/integrations/native_app_start_integration.dart
@github-actions

github-actions Bot commented Nov 18, 2024 •
edited
Loading

Copy link
Copy Markdown
Contributor

iOS Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1254.24 ms 1274.58 ms 20.34 ms
Size 8.38 MiB 9.77 MiB 1.40 MiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
824df58 1235.72 ms 1259.02 ms 23.30 ms
3f3ef0b 1223.73 ms 1237.67 ms 13.94 ms
4c78360 1230.35 ms 1252.37 ms 22.01 ms
ca9f398 1227.29 ms 1236.20 ms 8.92 ms
dd933d4 1238.73 ms 1252.43 ms 13.70 ms
afa6e2a 1251.04 ms 1266.65 ms 15.61 ms
a49594a 1284.83 ms 1313.29 ms 28.45 ms
a094100 1260.27 ms 1276.75 ms 16.48 ms
8fa3934 1252.79 ms 1272.41 ms 19.62 ms
613760b 1263.10 ms 1277.27 ms 14.16 ms

App size

Revision Plain With Sentry Diff
824df58 8.33 MiB 9.64 MiB 1.31 MiB
3f3ef0b 8.32 MiB 9.38 MiB 1.05 MiB
4c78360 8.32 MiB 9.38 MiB 1.06 MiB
ca9f398 8.38 MiB 9.74 MiB 1.36 MiB
dd933d4 8.33 MiB 9.64 MiB 1.31 MiB
afa6e2a 8.28 MiB 9.33 MiB 1.05 MiB
a49594a 8.16 MiB 9.16 MiB 1.00 MiB
a094100 8.16 MiB 9.17 MiB 1.01 MiB
8fa3934 8.09 MiB 9.07 MiB 1000.86 KiB
613760b 8.15 MiB 9.13 MiB 1000.46 KiB

Previous results on branch: enh/app-start-time

Startup times

Revision Plain With Sentry Diff
d724aac 1246.35 ms 1271.84 ms 25.48 ms
9be4829 1224.19 ms 1243.20 ms 19.01 ms
b1e9b9c 1252.49 ms 1273.52 ms 21.03 ms
803684b 1253.82 ms 1263.78 ms 9.96 ms
34988b1 1252.16 ms 1268.30 ms 16.13 ms
5990562 1245.67 ms 1275.57 ms 29.90 ms
4def0ce 1252.67 ms 1274.83 ms 22.16 ms
1fec51e 1243.58 ms 1275.52 ms 31.94 ms
92b5b84 1233.21 ms 1244.96 ms 11.75 ms
88bc8a5 1242.29 ms 1264.04 ms 21.75 ms

App size

Revision Plain With Sentry Diff
d724aac 8.38 MiB 9.77 MiB 1.40 MiB
9be4829 8.38 MiB 9.77 MiB 1.40 MiB
b1e9b9c 8.38 MiB 9.77 MiB 1.40 MiB
803684b 8.38 MiB 9.77 MiB 1.40 MiB
34988b1 8.38 MiB 9.77 MiB 1.40 MiB
5990562 8.38 MiB 9.77 MiB 1.40 MiB
4def0ce 8.38 MiB 9.77 MiB 1.40 MiB
1fec51e 8.38 MiB 9.77 MiB 1.40 MiB
92b5b84 8.38 MiB 9.77 MiB 1.40 MiB
88bc8a5 8.38 MiB 9.77 MiB 1.40 MiB

@buenaflor
buenaflor merged commit 07cd9e8 into main Nov 20, 2024
@buenaflor
buenaflor deleted the enh/app-start-time branch November 20, 2024 17:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@denrase denrase denrase approved these changes
@krystofwoldrich krystofwoldrich Awaiting requested review from krystofwoldrich
@stefanosiano stefanosiano Awaiting requested review from stefanosiano
@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 @denrase