Streamline frame timings recording #25892

iskakaushik · 2021-05-03T18:23:19Z

FlutterFrameTimingsRecorder is passed through the lifecycle of the
frame and records the events that happen through it. This also serves as
a builder for the FrameTiming object that is later passed to the
framework.

This change also aims to lay the foundation to capture a unique frame
identifier which will later be added to FlutterFrameTimingsRecorder to
identify the trace events corresponding to each frame.

x-ref: #25662
x-ref: flutter/flutter#80735

iskakaushik · 2021-05-03T18:26:40Z

@jonahwilliams let me know what you think, it should be fairly easy to track additional information like frame_id / frame_key once we have this. I think something like:

Increment the frame_id on vsync.
Associate the id with frame timings recorder.
Add that to the trace events.

should work.

Note: If you are OK with the overall approach, I will add docs and tests :-)

jonahwilliams

Nice!

Would it be the responsibility of the different parts of the engine code here to associate each trace event with the frame key from the FrameTimingRecorder?

iskakaushik · 2021-05-03T19:31:05Z

Would it be the responsibility of the different parts of the engine code here to associate each trace event with the frame key from the FrameTimingRecorder?

That is the idea. Until now there hasn't been an easy way to do that, but this should make it easy.

jonahwilliams · 2021-05-03T19:31:50Z

SGTM1

iskakaushik · 2021-05-04T15:06:17Z

@jonahwilliams , @chinmaygarde this is ready for review now.

chinmaygarde

Sound direction. A few suggestions.

flow/frame_timings.h

chinmaygarde · 2021-05-04T17:46:45Z

flow/frame_timings.cc

+
+fml::TimePoint FrameTimingsRecorder::GetVsyncStartTime() const {
+  std::scoped_lock state_lock(state_mutex_);
+  FML_CHECK(state_ >= State::kVsync);


I am nervous about incorrect use of the frame timings recorder taking down the process. Its hardly the mission critical.

But, instead of converting this to a DCHECK, there may be opportunities to make this API harder to misuse.

How about a FrameTimingsBuilder where one adds the various times as they are obtained. Once the build method on the builder is called, only a valid frame timings object may be returned with no checks necessary during use.

Generally, we can use sound API design to guard against API misuse and asserts to guard against construction issues. I feel we may be using asserts for the former here.

This class was meant to be the equivalent of FrameTimingsBuider. I will convert the checks to DCHECKs. Build method here is RecordRasterEnd which returns a FrameTimings object. I absolutely agree with you that I was using FML_CHECK too aggressively.

flow/frame_timings.h

fluttergithubbot · 2021-05-06T21:44:14Z

This pull request is not suitable for automatic merging in its current state.

The status or check suite Windows Android AOT Engine has failed. Please fix the issues identified (or deflake) before re-applying this label.
The status or check suite Windows Host Engine has failed. Please fix the issues identified (or deflake) before re-applying this label.
The status or check suite Windows UWP Engine has failed. Please fix the issues identified (or deflake) before re-applying this label.

fluttergithubbot · 2021-05-07T15:54:02Z

This pull request is not suitable for automatic merging in its current state.

The status or check suite Windows UWP Engine has failed. Please fix the issues identified (or deflake) before re-applying this label.
The status or check suite Windows Host Engine has failed. Please fix the issues identified (or deflake) before re-applying this label.
The status or check suite Windows Android AOT Engine has failed. Please fix the issues identified (or deflake) before re-applying this label.

`FlutterFrameTimingsRecorder` is passed through the lifecycle of the frame and records the events that happen through it. This also serves as a builder for the `FrameTiming` object that is later passed to the framework. This change also aims to lay the foundation to capture a unique frame identifier which will later be added to `FlutterFrameTimingsRecorder` to identify the trace events corresponding to each frame. x-ref: flutter#25662 x-ref: flutter/flutter#80735

* ccaae8d Roll Skia from 537293bf155f to adadb95a9f1e (1 revision) (flutter/engine#25905) * 2e3a7f8 Roll buildroot to pick change to cppwinrt invocation (flutter/engine#25993) * 1c22286 [Linux] revise dark theme detection (flutter/engine#25535) * 7424400 Moved PlatformMessage's to unique_ptrs (flutter/engine#25860) * 406c4da Ensure that AutoIsolateShutdown drops its reference to the DartIsolate on the intended task runner (flutter/engine#25899) * 72c2fda [web] Fix incorrect physical size due to visualviewport api on iOS (flutter/engine#25895) * a712ffe Roll Dart SDK from b8f4018535fa to 86c749398b3a (16 revisions) (flutter/engine#25999) * edbbb12 Roll Fuchsia Mac SDK from uQgs5ZmFq... to aCsEHpnS0... (flutter/engine#26001) * afbbeac Implement smooth resizing for Linux (flutter/engine#25884) * 3ffb8ef Roll Dart SDK from 86c749398b3a to b4210cc43086 (2 revisions) (flutter/engine#26006) * 039dcd9 pull googletest from github instead of fuchsia.googlesource (flutter/engine#25907) * 9c793f1 Roll Skia from adadb95a9f1e to 1dc2d0fe0fa0 (98 revisions) (flutter/engine#26009) * 82efb9a Roll Skia from 1dc2d0fe0fa0 to 115645ee9b1b (2 revisions) (flutter/engine#26012) * 91a4c72 Delete unused method from engine_layer.h (flutter/engine#25924) * 619f82f fuchsia: Fix multi-views fallout (flutter/engine#25984) * 0b4bf7e Fixes BUILD.gn if is_fuchsia (legacy embedder) and is_debug (flutter/engine#25858) * 2e9de09 Streamline frame timings recording (flutter/engine#25892) * 1077da8 [vsync_waiter] add AwaitVSyncForSecondaryCallback() (flutter/engine#25787) * 0053bef [build_fuchsia_artifacts] Move license copying into BuildBucket(). (flutter/engine#25815) * e8b80e7 Roll Skia from 115645ee9b1b to c411429239e9 (7 revisions) (flutter/engine#26018) * d3353b2 Move more parts to libraries. (flutter/engine#25863) * d1a1182 Roll Dart SDK from b4210cc43086 to 04e55dad908d (2 revisions) (flutter/engine#26020) * f631f5b Exclude third_party/dart/third_party/devtools from the license script (flutter/engine#25918) * 304539b Roll Fuchsia Mac SDK from aCsEHpnS0... to OyXxehV6e... (flutter/engine#26022) * c3e28ae Roll Fuchsia Linux SDK from 4numS0K6T... to -FIIsjZj2... (flutter/engine#26023) * 6cacb53 Roll Skia from c411429239e9 to 72de83df3a03 (1 revision) (flutter/engine#26024) * bfccba5 Add dependency on Windows SDK CIPD package (flutter/engine#26003) * 06cbf1e Roll Dart SDK from 04e55dad908d to 094c9024373c (1 revision) (flutter/engine#26027) * 73aaeea Roll Skia from 72de83df3a03 to dabb2891c4a1 (4 revisions) (flutter/engine#26025) * e229649 Extract Windows string_conversion target (flutter/engine#26029) * 3d73e06 Extract FML command_line target (flutter/engine#26028) * 4aace54 Make SceneBuilder.push* not return nullable objects (flutter/engine#25991) * 0d62a56 Roll Dart SDK from 094c9024373c to 2ea89ef8f6de (1 revision) (flutter/engine#26030) * 5a26c1f Use string_view inputs for conversion functions (flutter/engine#26031) * 4f28de4 Support windows registry access (flutter/engine#26032) * 234fae1 Roll Fuchsia Linux SDK from -FIIsjZj2... to KZCe5FqMb... (flutter/engine#26034) * 7a47d3b Roll Fuchsia Mac SDK from OyXxehV6e... to DSk0IzBHv... (flutter/engine#26035) * 01f9bd8 Roll Skia from dabb2891c4a1 to 686dd910dd6c (1 revision) (flutter/engine#26036) * 1825bef Revert Dart SDK to b8f4018535fa792891e2add3a475f35e3ec156ab (flutter/engine#26037)

iskakaushik requested a review from jonahwilliams May 3, 2021 18:23

google-cla bot added the cla: yes label May 3, 2021

jonahwilliams reviewed May 3, 2021

View reviewed changes

iskakaushik force-pushed the vsync_frame_info branch 5 times, most recently from c5a9bca to cedb4a3 Compare May 3, 2021 23:22

iskakaushik requested a review from chinmaygarde May 4, 2021 15:05

iskakaushik force-pushed the vsync_frame_info branch 2 times, most recently from 8f97df5 to e6c1beb Compare May 4, 2021 15:53

chinmaygarde reviewed May 4, 2021

View reviewed changes

iskakaushik force-pushed the vsync_frame_info branch from e6c1beb to 28cf41b Compare May 4, 2021 18:26

chinmaygarde approved these changes May 4, 2021

View reviewed changes

iskakaushik force-pushed the vsync_frame_info branch from 2e277c9 to 819a6cc Compare May 4, 2021 20:06

chinmaygarde added the waiting for tree to go green This PR is approved and tested, but waiting for the tree to be green to land. label May 6, 2021

fluttergithubbot removed the waiting for tree to go green This PR is approved and tested, but waiting for the tree to be green to land. label May 6, 2021

iskakaushik added the waiting for tree to go green This PR is approved and tested, but waiting for the tree to be green to land. label May 7, 2021

fluttergithubbot removed the waiting for tree to go green This PR is approved and tested, but waiting for the tree to be green to land. label May 7, 2021

iskakaushik added 5 commits May 7, 2021 10:07

add comments

9f6e883

add tests

31429cd

fix enum

cc4bd6d

account for prebuilt scenes

0278058

iskakaushik added the waiting for tree to go green This PR is approved and tested, but waiting for the tree to be green to land. label May 7, 2021

fix ifdef

502eb2c

fluttergithubbot merged commit 2e9de09 into flutter:master May 7, 2021

engine-flutter-autoroll mentioned this pull request May 7, 2021

Roll Engine from f57e986aa80c to f631f5bcb2d7 (23 revisions) flutter/flutter#82070

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request May 7, 2021

2e9de09 Streamline frame timings recording (flutter/engine#25892)

2a2caf8

engine-flutter-autoroll mentioned this pull request May 8, 2021

Roll Engine from f57e986aa80c to bfccba5ac00c (27 revisions) flutter/flutter#82079

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request May 8, 2021

2e9de09 Streamline frame timings recording (flutter/engine#25892)

6a2869f

engine-flutter-autoroll mentioned this pull request May 8, 2021

Roll Engine from f57e986aa80c to 06cbf1e0d773 (28 revisions) flutter/flutter#82083

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request May 8, 2021

2e9de09 Streamline frame timings recording (flutter/engine#25892)

9d427e0

engine-flutter-autoroll mentioned this pull request May 8, 2021

Roll Engine from f57e986aa80c to 73aaeea34640 (29 revisions) flutter/flutter#82090

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request May 8, 2021

2e9de09 Streamline frame timings recording (flutter/engine#25892)

a2bbe04

engine-flutter-autoroll mentioned this pull request May 8, 2021

Roll Engine from f57e986aa80c to e229649a7a97 (30 revisions) flutter/flutter#82096

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request May 8, 2021

2e9de09 Streamline frame timings recording (flutter/engine#25892)

f6d3e1f

engine-flutter-autoroll mentioned this pull request May 8, 2021

Roll Engine from f57e986aa80c to 4aace5476b33 (32 revisions) flutter/flutter#82097

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request May 8, 2021

2e9de09 Streamline frame timings recording (flutter/engine#25892)

a6d985b

engine-flutter-autoroll mentioned this pull request May 8, 2021

Roll Engine from f57e986aa80c to 0d62a560fe6a (33 revisions) flutter/flutter#82120

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request May 8, 2021

2e9de09 Streamline frame timings recording (flutter/engine#25892)

c79d90e

engine-flutter-autoroll mentioned this pull request May 8, 2021

Roll Engine from f57e986aa80c to 4f28de494253 (35 revisions) flutter/flutter#82121

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request May 8, 2021

2e9de09 Streamline frame timings recording (flutter/engine#25892)

1fa11c6

engine-flutter-autoroll mentioned this pull request May 8, 2021

Roll Engine from f57e986aa80c to 234fae1b8d1d (36 revisions) flutter/flutter#82122

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request May 8, 2021

2e9de09 Streamline frame timings recording (flutter/engine#25892)

f7d5e53

engine-flutter-autoroll mentioned this pull request May 8, 2021

Roll Engine from f57e986aa80c to 7a47d3b32567 (37 revisions) flutter/flutter#82127

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request May 8, 2021

2e9de09 Streamline frame timings recording (flutter/engine#25892)

b372961

engine-flutter-autoroll mentioned this pull request May 9, 2021

Roll Engine from f57e986aa80c to 01f9bd8649cd (38 revisions) flutter/flutter#82130

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request May 9, 2021

2e9de09 Streamline frame timings recording (flutter/engine#25892)

25808bb

engine-flutter-autoroll mentioned this pull request May 9, 2021

Roll Engine from f57e986aa80c to 1825befbc72d (39 revisions) flutter/flutter#82132

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request May 9, 2021

2e9de09 Streamline frame timings recording (flutter/engine#25892)

f5ee09d

iskakaushik mentioned this pull request May 12, 2021

Add flutter frame identifier to timeline events on UI and Raster threads flutter/flutter#80735

Closed

naudzghebre pushed a commit to naudzghebre/engine that referenced this pull request May 21, 2021

Streamline frame timings recording (flutter#25892)

0abfe00

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Streamline frame timings recording #25892

Streamline frame timings recording #25892

Uh oh!

iskakaushik commented May 3, 2021

Uh oh!

iskakaushik commented May 3, 2021 •

edited

Loading

Uh oh!

jonahwilliams left a comment

Uh oh!

iskakaushik commented May 3, 2021

Uh oh!

jonahwilliams commented May 3, 2021

Uh oh!

iskakaushik commented May 4, 2021

Uh oh!

chinmaygarde left a comment

Uh oh!

Uh oh!

Uh oh!

chinmaygarde May 4, 2021

Uh oh!

iskakaushik May 4, 2021

Uh oh!

Uh oh!

fluttergithubbot commented May 6, 2021

Uh oh!

fluttergithubbot commented May 7, 2021

Uh oh!

Uh oh!

Streamline frame timings recording #25892

Streamline frame timings recording #25892

Uh oh!

Conversation

iskakaushik commented May 3, 2021

Uh oh!

iskakaushik commented May 3, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jonahwilliams left a comment

Choose a reason for hiding this comment

Uh oh!

iskakaushik commented May 3, 2021

Uh oh!

jonahwilliams commented May 3, 2021

Uh oh!

iskakaushik commented May 4, 2021

Uh oh!

chinmaygarde left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

chinmaygarde May 4, 2021

Choose a reason for hiding this comment

Uh oh!

iskakaushik May 4, 2021

Choose a reason for hiding this comment

Uh oh!

Uh oh!

fluttergithubbot commented May 6, 2021

Uh oh!

fluttergithubbot commented May 7, 2021

Uh oh!

Uh oh!

iskakaushik commented May 3, 2021 •

edited

Loading