feat(performance): report total frames, frame delay, slow & frozen frames by buenaflor · Pull Request #2106 · getsentry/sentry-dart

buenaflor · 2024-06-17T13:20:41Z

📜 Description

Attaches frame metrics to spans such as total frames, frame delay, slow & frozen frames

💡 Motivation and Context

Mobile Starfish

💚 How did you test it?

Unit tests, manually

📝 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

### Tasks
- [ ] Update docs
- [x] Create issue that Linux, Windows and Web refresh rates are currently locked at 60hz for frame metrics calculation

github-actions · 2024-06-17T13:21:22Z

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

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

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

- report total frames, frame delay, slow & frozen frames ([#2106](https://github.com/getsentry/sentry-dart/pull/2106))

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 736bafc

github-actions · 2024-06-17T13:35:13Z

Android Performance metrics 🚀

	Plain	With Sentry	Diff
Startup time	353.21 ms	410.35 ms	57.13 ms
Size	6.35 MiB	7.35 MiB	1017.84 KiB

Baseline results on branch: main

Startup times

Revision	Plain	With Sentry	Diff
`f275487`	369.08 ms	432.44 ms	63.36 ms
`8776cdf`	353.32 ms	416.58 ms	63.26 ms
`42f6e7e`	308.71 ms	360.06 ms	51.35 ms
`04db237`	330.16 ms	428.38 ms	98.22 ms
`1c6eb5b`	350.69 ms	393.86 ms	43.17 ms
`ccc09e4`	308.21 ms	357.74 ms	49.54 ms
`e5b744f`	302.70 ms	342.17 ms	39.47 ms
`43abc4f`	406.54 ms	475.53 ms	68.99 ms
`64e4616`	349.82 ms	436.96 ms	87.14 ms
`56810ff`	309.72 ms	352.26 ms	42.54 ms

App size

Revision	Plain	With Sentry	Diff
`f275487`	6.33 MiB	7.26 MiB	947.03 KiB
`8776cdf`	6.34 MiB	7.28 MiB	966.66 KiB
`42f6e7e`	6.27 MiB	7.20 MiB	956.06 KiB
`04db237`	5.94 MiB	6.95 MiB	1.01 MiB
`1c6eb5b`	5.94 MiB	6.92 MiB	1001.53 KiB
`ccc09e4`	5.94 MiB	6.95 MiB	1.01 MiB
`e5b744f`	6.06 MiB	7.09 MiB	1.03 MiB
`43abc4f`	6.35 MiB	7.34 MiB	1007.72 KiB
`64e4616`	6.27 MiB	7.20 MiB	956.52 KiB
`56810ff`	5.94 MiB	6.92 MiB	1001.71 KiB

Previous results on branch: feat/frame-duration

Startup times

Revision	Plain	With Sentry	Diff
`2219ec3`	394.26 ms	451.51 ms	57.25 ms
`cb8e94f`	368.41 ms	433.87 ms	65.47 ms
`28de2ed`	407.94 ms	487.00 ms	79.06 ms
`71cdcdb`	363.48 ms	429.50 ms	66.02 ms
`b5630cb`	367.54 ms	387.82 ms	20.28 ms
`cf7c96c`	383.45 ms	440.74 ms	57.30 ms

App size

Revision	Plain	With Sentry	Diff
`2219ec3`	6.35 MiB	7.34 MiB	1013.39 KiB
`cb8e94f`	6.35 MiB	7.34 MiB	1013.39 KiB
`28de2ed`	6.35 MiB	7.35 MiB	1017.84 KiB
`71cdcdb`	6.35 MiB	7.35 MiB	1017.84 KiB
`b5630cb`	6.35 MiB	7.35 MiB	1017.84 KiB
`cf7c96c`	6.35 MiB	7.34 MiB	1013.40 KiB

codecov · 2024-06-18T11:41:31Z

Codecov Report

Attention: Patch coverage is 74.40000% with 32 lines in your changes missing coverage. Please review.

Project coverage is 88.41%. Comparing base (7e7f0b1) to head (736bafc).

Files	Patch %	Lines
flutter/lib/src/span_frame_metrics_collector.dart	86.31%	13 Missing ⚠️
flutter/lib/src/frame_callback_handler.dart	0.00%	6 Missing ⚠️
dart/lib/src/protocol/sentry_span.dart	54.54%	5 Missing ⚠️
dart/lib/src/sentry_tracer.dart	33.33%	4 Missing ⚠️
dart/lib/src/sentry_options.dart	50.00%	2 Missing ⚠️
flutter/lib/src/native/sentry_native_channel.dart	0.00%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2106      +/-   ##
==========================================
- Coverage   88.64%   88.41%   -0.24%     
==========================================
  Files         223      224       +1     
  Lines        7583     7705     +122     
==========================================
+ Hits         6722     6812      +90     
- Misses        861      893      +32

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

buenaflor · 2024-06-24T10:30:29Z

@stefanosiano I updated the cocoa refresh rate fetching so it also works on macos. Linux, windows and web don't have specific refresh rate fetching yet so it stays at 60hz for them and I don't think they're a priority currently. Maybe we can create an issue afterwards so we have it tracked at least?

stefanosiano · 2024-06-24T10:45:16Z

@stefanosiano I updated the cocoa refresh rate fetching so it also works on macos. Linux, windows and web don't have specific refresh rate fetching yet so it stays at 60hz for them and I don't think they're a priority currently. Maybe we can create an issue afterwards so we have it tracked at least?

yep, totally fine!

github-actions · 2024-06-24T10:56:11Z

iOS Performance metrics 🚀

	Plain	With Sentry	Diff
Startup time	1212.69 ms	1240.47 ms	27.78 ms
Size	8.33 MiB	9.62 MiB	1.29 MiB

Baseline results on branch: main

Startup times

Revision	Plain	With Sentry	Diff
`a609134`	1254.50 ms	1265.08 ms	10.58 ms
`ddc97ad`	1228.02 ms	1232.61 ms	4.59 ms
`9fe67d5`	1242.33 ms	1268.77 ms	26.44 ms
`95d0636`	1243.73 ms	1245.54 ms	1.81 ms
`bd37365`	1236.22 ms	1243.27 ms	7.04 ms
`b0811cc`	1238.23 ms	1255.82 ms	17.59 ms
`689d2fd`	1257.71 ms	1265.16 ms	7.45 ms
`3334ac1`	1259.22 ms	1275.40 ms	16.17 ms
`4efee31`	1270.33 ms	1285.75 ms	15.42 ms
`ae02632`	1286.77 ms	1300.37 ms	13.60 ms

App size

Revision	Plain	With Sentry	Diff
`a609134`	8.16 MiB	9.16 MiB	1.01 MiB
`ddc97ad`	8.29 MiB	9.37 MiB	1.08 MiB
`9fe67d5`	8.32 MiB	9.50 MiB	1.18 MiB
`95d0636`	8.29 MiB	9.38 MiB	1.09 MiB
`bd37365`	8.28 MiB	9.34 MiB	1.06 MiB
`b0811cc`	8.32 MiB	9.38 MiB	1.06 MiB
`689d2fd`	8.10 MiB	9.16 MiB	1.06 MiB
`3334ac1`	8.10 MiB	9.17 MiB	1.08 MiB
`4efee31`	8.15 MiB	9.12 MiB	991.35 KiB
`ae02632`	8.16 MiB	9.15 MiB	1020.68 KiB

Previous results on branch: feat/frame-duration

Startup times

Revision	Plain	With Sentry	Diff
`cb8e94f`	1227.33 ms	1260.00 ms	32.67 ms
`71cdcdb`	1345.35 ms	1397.72 ms	52.37 ms
`cf7c96c`	1218.04 ms	1230.35 ms	12.31 ms
`28de2ed`	1241.15 ms	1254.57 ms	13.42 ms
`2219ec3`	1207.22 ms	1230.77 ms	23.55 ms
`b5630cb`	1241.14 ms	1254.73 ms	13.59 ms

App size

Revision	Plain	With Sentry	Diff
`cb8e94f`	8.33 MiB	9.59 MiB	1.26 MiB
`71cdcdb`	8.33 MiB	9.62 MiB	1.29 MiB
`cf7c96c`	8.33 MiB	9.59 MiB	1.26 MiB
`28de2ed`	8.33 MiB	9.62 MiB	1.29 MiB
`2219ec3`	8.33 MiB	9.59 MiB	1.26 MiB
`b5630cb`	8.33 MiB	9.62 MiB	1.29 MiB

stefanosiano

Guess we are good. Nice!

buenaflor · 2024-06-24T17:01:56Z

@stefanosiano seems like CVDisplayLink which I'm using for the macos refresh rate impl is deprecated, I'll try to migrate it to the recommended API

buenaflor · 2024-06-25T11:30:52Z

Updated the implementation, works nicely and detects the refresh rate depending on the screen where the window is currently at

FujiKinaga · 2024-08-16T01:27:06Z

+      _stopwatch.start();
+    }
+
+    await _frameCallbackHandler.endOfFrame;


@buenaflor
Sorry for commenting after merge pr🙏
If you use "await" in a FrameCallback function, is there any possibility of delaying all actions?

hi, what do you mean by delaying all actions? do you mean blocking?

@buenaflor
In our product, we experienced an issue with delayed screen transition behaviour, which improved when enableFramesTracking was set to false. This is probably due to blocked processing in the FrameCallback.

At 60 fps, this callback is called up to 60 times per second.
You should not use await in FrameCallback!

I see, do you have a way to reproduce this in a sample app?

I'll have a look how we can optimize this

At 60 fps, this callback is called up to 60 times per second. You should not use await in FrameCallback!

this is not the reason. using await here is fine.

See this issue for info why this is happening

buenaflor added 7 commits June 11, 2024 18:52

Add frame tracking option

849df8e

Current state

633047b

Update

cf7c96c

Update

cf6a130

Update

cb8e94f

Update

2219ec3

Update

0880d0e

buenaflor added 3 commits June 17, 2024 15:23

Merge remote-tracking branch 'origin/main' into feat/frame-duration

be90b2e

Update Changelog

60f62b6

Update

7d22941

update

f116f40

buenaflor commented Jun 18, 2024

View reviewed changes

Comment thread dart/lib/src/protocol/sentry_span.dart

buenaflor added 5 commits June 18, 2024 13:09

Format

7d2c89f

Update

da91065

Remove _isFinished

01fa33d

clean up

140d2e1

format

019feed

buenaflor added 2 commits June 18, 2024 13:43

Clean up

38aa0d9

Add native channel to span frame collector

e88aa35

github-actions Bot reviewed Jun 18, 2024

View reviewed changes

Comment thread flutter/android/src/main/kotlin/io/sentry/flutter/SentryFlutterPlugin.kt Outdated

buenaflor added 7 commits June 18, 2024 16:12

Clean up

a719da2

Update

0e4d02c

Fix has scheduled frame

90d5f2b

Clean up

5ef1449

Clean up

3ad6033

Update

9aaaf4d

Format

89f0e18

buenaflor added 3 commits June 24, 2024 12:10

Fix ktlint

1f4e58f

fix dart analyze

1cecda2

Fix analyze

74a6d33

buenaflor requested a review from stefanosiano June 24, 2024 10:33

Merge branch 'main' into feat/frame-duration

b5630cb