From 42e24ed927816df3184ccdda9eec08cf1fd1bffd Mon Sep 17 00:00:00 2001 From: Jonah Williams Date: Mon, 13 Nov 2023 09:53:52 -0800 Subject: [PATCH] [engine] request frame rate once per frame. (flutter/engine#47954) With the current stopwatch design, we request the frame rate multiple times per frame. On Android this calls into JNI which is pretty slow. If we cache the value at construction of the StopwatchVisualizer, then we will only compute it once per frame (because the StopwatchVisualizer is reconstructed each frame). Fixes https://github.com/flutter/flutter/issues/137797 So the issue isn't that we're checking the fresh rate every frame, its that we were checking N times on each frame. --- engine/src/flutter/flow/stopwatch.cc | 2 +- engine/src/flutter/flow/stopwatch.h | 10 +++++++++- engine/src/flutter/flow/stopwatch_dl.cc | 2 +- engine/src/flutter/flow/stopwatch_sk.cc | 4 ++-- engine/src/flutter/flow/stopwatch_unittests.cc | 17 +++++++++++------ 5 files changed, 24 insertions(+), 11 deletions(-) diff --git a/engine/src/flutter/flow/stopwatch.cc b/engine/src/flutter/flow/stopwatch.cc index d61a591bd1c..1319959dd80 100644 --- a/engine/src/flutter/flow/stopwatch.cc +++ b/engine/src/flutter/flow/stopwatch.cc @@ -57,7 +57,7 @@ size_t Stopwatch::GetCurrentSample() const { } double StopwatchVisualizer::UnitFrameInterval(double raster_time_ms) const { - return raster_time_ms / stopwatch_.GetFrameBudget().count(); + return raster_time_ms / frame_budget_.count(); } double StopwatchVisualizer::UnitHeight(double raster_time_ms, diff --git a/engine/src/flutter/flow/stopwatch.h b/engine/src/flutter/flow/stopwatch.h index 5755b8d460b..cae8a6fa2c8 100644 --- a/engine/src/flutter/flow/stopwatch.h +++ b/engine/src/flutter/flow/stopwatch.h @@ -93,7 +93,12 @@ class FixedRefreshRateStopwatch : public Stopwatch { class StopwatchVisualizer { public: explicit StopwatchVisualizer(const Stopwatch& stopwatch) - : stopwatch_(stopwatch) {} + : stopwatch_(stopwatch) { + // Looking up the frame budget from the stopwatch delegate class may call + // into JNI or make platform calls which are slow. This value is safe to + // cache since the StopwatchVisualizer is recreated on each frame. + frame_budget_ = stopwatch_.GetFrameBudget(); + } virtual ~StopwatchVisualizer() = default; @@ -112,7 +117,10 @@ class StopwatchVisualizer { /// @brief Converts a raster time to a unit height. double UnitHeight(double time_ms, double max_height) const; + fml::Milliseconds GetFrameBudget() const { return frame_budget_; } + const Stopwatch& stopwatch_; + fml::Milliseconds frame_budget_; }; } // namespace flutter diff --git a/engine/src/flutter/flow/stopwatch_dl.cc b/engine/src/flutter/flow/stopwatch_dl.cc index 8c20b035e53..9f7fa3f37ae 100644 --- a/engine/src/flutter/flow/stopwatch_dl.cc +++ b/engine/src/flutter/flow/stopwatch_dl.cc @@ -30,7 +30,7 @@ void DlStopwatchVisualizer::Visualize(DlCanvas* canvas, auto const bottom = rect.bottom(); // Scale the graph to show time frames up to those that are 3x the frame time. - auto const one_frame_ms = stopwatch_.GetFrameBudget().count(); + auto const one_frame_ms = GetFrameBudget().count(); auto const max_interval = one_frame_ms * 3.0; auto const max_unit_interval = UnitFrameInterval(max_interval); auto const sample_unit_width = (1.0 / kMaxSamples); diff --git a/engine/src/flutter/flow/stopwatch_sk.cc b/engine/src/flutter/flow/stopwatch_sk.cc index 16a3411fdf9..91d5bf94b2a 100644 --- a/engine/src/flutter/flow/stopwatch_sk.cc +++ b/engine/src/flutter/flow/stopwatch_sk.cc @@ -47,7 +47,7 @@ void SkStopwatchVisualizer::InitVisualizeSurface(SkISize size) const { // Scale the graph to show frame times up to those that are 3 times the frame // time. - const double one_frame_ms = stopwatch_.GetFrameBudget().count(); + const double one_frame_ms = GetFrameBudget().count(); const double max_interval = one_frame_ms * 3.0; const double max_unit_interval = UnitFrameInterval(max_interval); @@ -101,7 +101,7 @@ void SkStopwatchVisualizer::Visualize(DlCanvas* canvas, // Scale the graph to show frame times up to those that are 3 times the frame // time. - const double one_frame_ms = stopwatch_.GetFrameBudget().count(); + const double one_frame_ms = GetFrameBudget().count(); const double max_interval = one_frame_ms * 3.0; const double max_unit_interval = UnitFrameInterval(max_interval); diff --git a/engine/src/flutter/flow/stopwatch_unittests.cc b/engine/src/flutter/flow/stopwatch_unittests.cc index f865b3831c3..c8fae3e0479 100644 --- a/engine/src/flutter/flow/stopwatch_unittests.cc +++ b/engine/src/flutter/flow/stopwatch_unittests.cc @@ -3,6 +3,7 @@ // found in the LICENSE file. #include "flutter/flow/stopwatch.h" +#include "fml/time/time_delta.h" #include "gmock/gmock.h" // IWYU pragma: keep #include "gtest/gtest.h" @@ -11,9 +12,14 @@ using testing::Return; namespace flutter { namespace testing { -class MockRefreshRateUpdater : public Stopwatch::RefreshRateUpdater { +class FakeRefreshRateUpdater : public Stopwatch::RefreshRateUpdater { public: - MOCK_METHOD(fml::Milliseconds, GetFrameBudget, (), (const, override)); + fml::Milliseconds GetFrameBudget() const override { return budget_; } + + void SetFrameBudget(fml::Milliseconds budget) { budget_ = budget; } + + private: + fml::Milliseconds budget_; }; TEST(Instrumentation, GetDefaultFrameBudgetTest) { @@ -32,11 +38,10 @@ TEST(Instrumentation, GetOneShotFrameBudgetTest) { } TEST(Instrumentation, GetFrameBudgetFromUpdaterTest) { - MockRefreshRateUpdater updater; + FakeRefreshRateUpdater updater; fml::Milliseconds frame_budget_90fps = fml::RefreshRateToFrameBudget(90); - EXPECT_CALL(updater, GetFrameBudget()) - .Times(1) - .WillOnce(Return(frame_budget_90fps)); + updater.SetFrameBudget(frame_budget_90fps); + Stopwatch stopwatch(updater); fml::Milliseconds actual_frame_budget = stopwatch.GetFrameBudget(); EXPECT_EQ(frame_budget_90fps, actual_frame_budget);