From 2c6dbee8dafda1056cb2fb6992f8e051701f97af Mon Sep 17 00:00:00 2001 From: Jonah Williams Date: Mon, 19 Apr 2021 16:21:54 -0700 Subject: [PATCH 1/3] Configure LayerTree with a framework provided frame key --- common/settings.h | 6 ++++++ flow/layers/layer_tree.h | 9 +++++++++ lib/ui/compositing.dart | 9 ++++++--- lib/ui/compositing/scene.cc | 9 ++++++--- lib/ui/compositing/scene.h | 6 ++++-- lib/ui/compositing/scene_builder.cc | 5 +++-- lib/ui/compositing/scene_builder.h | 2 +- lib/ui/platform_dispatcher.dart | 20 +++++++++++++------ .../engine/canvaskit/layer_scene_builder.dart | 2 +- .../lib/src/engine/html/scene_builder.dart | 2 +- lib/web_ui/lib/src/ui/compositing.dart | 2 +- shell/common/rasterizer.cc | 1 + shell/common/shell.cc | 5 +++-- 13 files changed, 56 insertions(+), 22 deletions(-) diff --git a/common/settings.h b/common/settings.h index d5304e47681f6..0bbdd6bfa1f65 100644 --- a/common/settings.h +++ b/common/settings.h @@ -40,8 +40,14 @@ class FrameTiming { return data_[phase] = value; } + int64_t GetFrameKey() const { return frame_key_; } + void SetFrameKey(int64_t value) { + frame_key_ = value; + } + private: fml::TimePoint data_[kCount]; + int64_t frame_key_; }; using TaskObserverAdd = diff --git a/flow/layers/layer_tree.h b/flow/layers/layer_tree.h index db8a33b4506c8..8af370674ef67 100644 --- a/flow/layers/layer_tree.h +++ b/flow/layers/layer_tree.h @@ -85,6 +85,14 @@ class LayerTree { checkerboard_offscreen_layers_ = checkerboard; } + void set_frame_key(int64_t frame_key) { + frame_key_ = frame_key; + } + + int64_t frame_key() const { + return frame_key_; + } + private: std::shared_ptr root_layer_; fml::TimePoint vsync_start_; @@ -96,6 +104,7 @@ class LayerTree { uint32_t rasterizer_tracing_threshold_; bool checkerboard_raster_cache_images_; bool checkerboard_offscreen_layers_; + int64_t frame_key_; #ifdef FLUTTER_ENABLE_DIFF_CONTEXT PaintRegionMap paint_region_map_; diff --git a/lib/ui/compositing.dart b/lib/ui/compositing.dart index 4f9e66fe257dd..92123a378d454 100644 --- a/lib/ui/compositing.dart +++ b/lib/ui/compositing.dart @@ -824,13 +824,16 @@ class SceneBuilder extends NativeFieldWrapperClass2 { /// /// After calling this function, the scene builder object is invalid and /// cannot be used further. - Scene build() { + /// + /// A [frameKey] may be optionally supplied to associate any [FrameTiming] + /// objects and/or timeline traces with a particular identifier. + Scene build({int? frameKey}) { final Scene scene = Scene._(); - _build(scene); + _build(scene, frameKey ?? -1); return scene; } - void _build(Scene outScene) native 'SceneBuilder_build'; + void _build(Scene outScene, int frameKey) native 'SceneBuilder_build'; } /// (Fuchsia-only) Hosts content provided by another application. diff --git a/lib/ui/compositing/scene.cc b/lib/ui/compositing/scene.cc index 46ac6efa534ca..dd762ee612e5a 100644 --- a/lib/ui/compositing/scene.cc +++ b/lib/ui/compositing/scene.cc @@ -31,17 +31,19 @@ void Scene::create(Dart_Handle scene_handle, std::shared_ptr rootLayer, uint32_t rasterizerTracingThreshold, bool checkerboardRasterCacheImages, - bool checkerboardOffscreenLayers) { + bool checkerboardOffscreenLayers, + int64_t frame_key) { auto scene = fml::MakeRefCounted( std::move(rootLayer), rasterizerTracingThreshold, - checkerboardRasterCacheImages, checkerboardOffscreenLayers); + checkerboardRasterCacheImages, checkerboardOffscreenLayers, frame_key); scene->AssociateWithDartWrapper(scene_handle); } Scene::Scene(std::shared_ptr rootLayer, uint32_t rasterizerTracingThreshold, bool checkerboardRasterCacheImages, - bool checkerboardOffscreenLayers) { + bool checkerboardOffscreenLayers, + int64_t frame_key) { // Currently only supports a single window. auto viewport_metrics = UIDartState::Current() ->platform_configuration() @@ -57,6 +59,7 @@ Scene::Scene(std::shared_ptr rootLayer, layer_tree_->set_checkerboard_raster_cache_images( checkerboardRasterCacheImages); layer_tree_->set_checkerboard_offscreen_layers(checkerboardOffscreenLayers); + layer_tree_->set_frame_key(frame_key); } Scene::~Scene() {} diff --git a/lib/ui/compositing/scene.h b/lib/ui/compositing/scene.h index f7a8d93711814..b0d3e094d07c1 100644 --- a/lib/ui/compositing/scene.h +++ b/lib/ui/compositing/scene.h @@ -28,7 +28,8 @@ class Scene : public RefCountedDartWrappable { std::shared_ptr rootLayer, uint32_t rasterizerTracingThreshold, bool checkerboardRasterCacheImages, - bool checkerboardOffscreenLayers); + bool checkerboardOffscreenLayers, + int64_t frame_key); std::unique_ptr takeLayerTree(); @@ -44,7 +45,8 @@ class Scene : public RefCountedDartWrappable { explicit Scene(std::shared_ptr rootLayer, uint32_t rasterizerTracingThreshold, bool checkerboardRasterCacheImages, - bool checkerboardOffscreenLayers); + bool checkerboardOffscreenLayers, + int64_t frame_key); std::unique_ptr layer_tree_; }; diff --git a/lib/ui/compositing/scene_builder.cc b/lib/ui/compositing/scene_builder.cc index 4acf0a3a1515c..57eb1c7f658a9 100644 --- a/lib/ui/compositing/scene_builder.cc +++ b/lib/ui/compositing/scene_builder.cc @@ -344,12 +344,13 @@ void SceneBuilder::setCheckerboardOffscreenLayers(bool checkerboard) { checkerboard_offscreen_layers_ = checkerboard; } -void SceneBuilder::build(Dart_Handle scene_handle) { +void SceneBuilder::build(Dart_Handle scene_handle, int64_t frame_key) { FML_DCHECK(layer_stack_.size() >= 1); Scene::create(scene_handle, layer_stack_[0], rasterizer_tracing_threshold_, checkerboard_raster_cache_images_, - checkerboard_offscreen_layers_); + checkerboard_offscreen_layers_, + frame_key); ClearDartWrapper(); // may delete this object. } diff --git a/lib/ui/compositing/scene_builder.h b/lib/ui/compositing/scene_builder.h index 70d37f6123559..677013851df0b 100644 --- a/lib/ui/compositing/scene_builder.h +++ b/lib/ui/compositing/scene_builder.h @@ -130,7 +130,7 @@ class SceneBuilder : public RefCountedDartWrappable { void setCheckerboardRasterCacheImages(bool checkerboard); void setCheckerboardOffscreenLayers(bool checkerboard); - void build(Dart_Handle scene_handle); + void build(Dart_Handle scene_handle, int64_t frame_key); static void RegisterNatives(tonic::DartLibraryNatives* natives); diff --git a/lib/ui/platform_dispatcher.dart b/lib/ui/platform_dispatcher.dart index 3efa757e6dae2..ec9c32ba1c5a3 100644 --- a/lib/ui/platform_dispatcher.dart +++ b/lib/ui/platform_dispatcher.dart @@ -433,10 +433,10 @@ class PlatformDispatcher { // Called from the engine, via hooks.dart void _reportTimings(List timings) { - assert(timings.length % FramePhase.values.length == 0); + assert(timings.length % (FramePhase.values.length + 1) == 0); final List frameTimings = []; - for (int i = 0; i < timings.length; i += FramePhase.values.length) { - frameTimings.add(FrameTiming._(timings.sublist(i, i + FramePhase.values.length))); + for (int i = 0; i < timings.length; i += FramePhase.values.length + 1) { + frameTimings.add(FrameTiming._(timings.sublist(i, i + FramePhase.values.length + 1))); } _invoke1(onReportTimings, _onReportTimingsZone, frameTimings); } @@ -1151,13 +1151,15 @@ class FrameTiming { required int buildFinish, required int rasterStart, required int rasterFinish, + required int frameKey, }) { return FrameTiming._([ vsyncStart, buildStart, buildFinish, rasterStart, - rasterFinish + rasterFinish, + frameKey, ]); } @@ -1169,7 +1171,7 @@ class FrameTiming { /// This constructor is usually only called by the Flutter engine, or a test. /// To get the [FrameTiming] of your app, see [PlatformDispatcher.onReportTimings]. FrameTiming._(this._timestamps) - : assert(_timestamps.length == FramePhase.values.length); + : assert(_timestamps.length == FramePhase.values.length + 1); /// This is a raw timestamp in microseconds from some epoch. The epoch in all /// [FrameTiming] is the same, but it may not match [DateTime]'s epoch. @@ -1214,13 +1216,19 @@ class FrameTiming { /// See also [vsyncOverhead], [buildDuration] and [rasterDuration]. Duration get totalSpan => _rawDuration(FramePhase.rasterFinish) - _rawDuration(FramePhase.vsyncStart); + /// The frame key associated with this frame measurement, or `-1` if nothing + /// was provided. + /// + /// This value should correspond the the frame key provided to [SceneBuilder.build]. + int get frameKey => _timestamps.last; + final List _timestamps; // in microseconds String _formatMS(Duration duration) => '${duration.inMicroseconds * 0.001}ms'; @override String toString() { - return '$runtimeType(buildDuration: ${_formatMS(buildDuration)}, rasterDuration: ${_formatMS(rasterDuration)}, vsyncOverhead: ${_formatMS(vsyncOverhead)}, totalSpan: ${_formatMS(totalSpan)})'; + return '$runtimeType(buildDuration: ${_formatMS(buildDuration)}, rasterDuration: ${_formatMS(rasterDuration)}, vsyncOverhead: ${_formatMS(vsyncOverhead)}, totalSpan: ${_formatMS(totalSpan)}, frameKey: ${_timestamps.last})'; } } diff --git a/lib/web_ui/lib/src/engine/canvaskit/layer_scene_builder.dart b/lib/web_ui/lib/src/engine/canvaskit/layer_scene_builder.dart index e022b0ba8de42..d54c238498bba 100644 --- a/lib/web_ui/lib/src/engine/canvaskit/layer_scene_builder.dart +++ b/lib/web_ui/lib/src/engine/canvaskit/layer_scene_builder.dart @@ -84,7 +84,7 @@ class LayerSceneBuilder implements ui.SceneBuilder { } @override - LayerScene build() { + LayerScene build({int? frameKey}) { return LayerScene(rootLayer); } diff --git a/lib/web_ui/lib/src/engine/html/scene_builder.dart b/lib/web_ui/lib/src/engine/html/scene_builder.dart index 99a4118563ed3..d55f218b8d36d 100644 --- a/lib/web_ui/lib/src/engine/html/scene_builder.dart +++ b/lib/web_ui/lib/src/engine/html/scene_builder.dart @@ -538,7 +538,7 @@ class SurfaceSceneBuilder implements ui.SceneBuilder { /// After calling this function, the scene builder object is invalid and /// cannot be used further. @override - SurfaceScene build() { + SurfaceScene build({int? frameKey}) { // "Build finish" and "raster start" happen back-to-back because we // render on the same thread, so there's no overhead from hopping to // another thread. diff --git a/lib/web_ui/lib/src/ui/compositing.dart b/lib/web_ui/lib/src/ui/compositing.dart index 5b84a2240d95d..3fa0329dbf2e2 100644 --- a/lib/web_ui/lib/src/ui/compositing.dart +++ b/lib/web_ui/lib/src/ui/compositing.dart @@ -129,7 +129,7 @@ abstract class SceneBuilder { void setRasterizerTracingThreshold(int frameInterval); void setCheckerboardRasterCacheImages(bool checkerboard); void setCheckerboardOffscreenLayers(bool checkerboard); - Scene build(); + Scene build({int? frameKey}); void setProperties( double width, double height, diff --git a/shell/common/rasterizer.cc b/shell/common/rasterizer.cc index acd33975f2254..0da794f7116ce 100644 --- a/shell/common/rasterizer.cc +++ b/shell/common/rasterizer.cc @@ -349,6 +349,7 @@ RasterStatus Rasterizer::DoDraw( timing.Set(FrameTiming::kBuildStart, layer_tree->build_start()); timing.Set(FrameTiming::kBuildFinish, layer_tree->build_finish()); timing.Set(FrameTiming::kRasterStart, fml::TimePoint::Now()); + timing.SetFrameKey(layer_tree->frame_key()); PersistentCache* persistent_cache = PersistentCache::GetCacheForProcess(); persistent_cache->ResetStoredNewShaders(); diff --git a/shell/common/shell.cc b/shell/common/shell.cc index e919c1b86a2f2..2d3ca7b3c4e2a 100644 --- a/shell/common/shell.cc +++ b/shell/common/shell.cc @@ -1353,8 +1353,8 @@ void Shell::ReportTimings() { size_t Shell::UnreportedFramesCount() const { // Check that this is running on the raster thread to avoid race conditions. FML_DCHECK(task_runners_.GetRasterTaskRunner()->RunsTasksOnCurrentThread()); - FML_DCHECK(unreported_timings_.size() % FrameTiming::kCount == 0); - return unreported_timings_.size() / FrameTiming::kCount; + FML_DCHECK(unreported_timings_.size() % (FrameTiming::kCount + 1) == 0); + return unreported_timings_.size() / (FrameTiming::kCount + 1); } void Shell::OnFrameRasterized(const FrameTiming& timing) { @@ -1375,6 +1375,7 @@ void Shell::OnFrameRasterized(const FrameTiming& timing) { unreported_timings_.push_back( timing.Get(phase).ToEpochDelta().ToMicroseconds()); } + unreported_timings_.push_back(timing.GetFrameKey()); // In tests using iPhone 6S with profile mode, sending a batch of 1 frame or a // batch of 100 frames have roughly the same cost of less than 0.1ms. Sending From dfa194adaab87d22ece0b9737def58e447cbfee6 Mon Sep 17 00:00:00 2001 From: Jonah Williams Date: Mon, 19 Apr 2021 16:51:21 -0700 Subject: [PATCH 2/3] apply clang format --- common/settings.h | 4 +--- flow/layers/layer_tree.h | 8 ++------ lib/ui/compositing/scene_builder.cc | 3 +-- 3 files changed, 4 insertions(+), 11 deletions(-) diff --git a/common/settings.h b/common/settings.h index 0bbdd6bfa1f65..e3f25f94d0bf8 100644 --- a/common/settings.h +++ b/common/settings.h @@ -41,9 +41,7 @@ class FrameTiming { } int64_t GetFrameKey() const { return frame_key_; } - void SetFrameKey(int64_t value) { - frame_key_ = value; - } + void SetFrameKey(int64_t value) { frame_key_ = value; } private: fml::TimePoint data_[kCount]; diff --git a/flow/layers/layer_tree.h b/flow/layers/layer_tree.h index 8af370674ef67..54b84758a05ae 100644 --- a/flow/layers/layer_tree.h +++ b/flow/layers/layer_tree.h @@ -85,13 +85,9 @@ class LayerTree { checkerboard_offscreen_layers_ = checkerboard; } - void set_frame_key(int64_t frame_key) { - frame_key_ = frame_key; - } + void set_frame_key(int64_t frame_key) { frame_key_ = frame_key; } - int64_t frame_key() const { - return frame_key_; - } + int64_t frame_key() const { return frame_key_; } private: std::shared_ptr root_layer_; diff --git a/lib/ui/compositing/scene_builder.cc b/lib/ui/compositing/scene_builder.cc index 57eb1c7f658a9..b781f05a50e03 100644 --- a/lib/ui/compositing/scene_builder.cc +++ b/lib/ui/compositing/scene_builder.cc @@ -349,8 +349,7 @@ void SceneBuilder::build(Dart_Handle scene_handle, int64_t frame_key) { Scene::create(scene_handle, layer_stack_[0], rasterizer_tracing_threshold_, checkerboard_raster_cache_images_, - checkerboard_offscreen_layers_, - frame_key); + checkerboard_offscreen_layers_, frame_key); ClearDartWrapper(); // may delete this object. } From 821af816b505e1f4a77745b09407fce6a482beb2 Mon Sep 17 00:00:00 2001 From: Jonah Williams Date: Tue, 20 Apr 2021 10:08:57 -0700 Subject: [PATCH 3/3] make frame key non-required to avoid breaking change --- lib/ui/platform_dispatcher.dart | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/ui/platform_dispatcher.dart b/lib/ui/platform_dispatcher.dart index ec9c32ba1c5a3..e48c1bf6cd518 100644 --- a/lib/ui/platform_dispatcher.dart +++ b/lib/ui/platform_dispatcher.dart @@ -1151,7 +1151,7 @@ class FrameTiming { required int buildFinish, required int rasterStart, required int rasterFinish, - required int frameKey, + int? frameKey, }) { return FrameTiming._([ vsyncStart, @@ -1159,7 +1159,7 @@ class FrameTiming { buildFinish, rasterStart, rasterFinish, - frameKey, + frameKey ?? -1, ]); }