From c58fbeb7a352d3c0c4fc10933f2c07c7cfe4e447 Mon Sep 17 00:00:00 2001 From: Felipe Archondo Date: Thu, 17 Sep 2020 15:41:03 -0400 Subject: [PATCH] [fuchsia] set maxframesinflight to be configurable (flutter/engine#21152) Add a new field, max_frames_in_flight, to FlutterRunnerProductConfiguration. This allows it to be configurable from Fuchsia. Tests were added as well to verify the new behavior. --- .../shell/platform/fuchsia/flutter/engine.cc | 4 +++- .../flutter_runner_product_configuration.cc | 3 +++ .../flutter_runner_product_configuration.h | 2 ++ .../fuchsia/flutter/session_connection.cc | 6 ++++-- .../fuchsia/flutter/session_connection.h | 5 +++-- ..._runner_product_configuration_unittests.cc | 20 +++++++++++++++++++ .../tests/session_connection_unittests.cc | 4 ++-- 7 files changed, 37 insertions(+), 7 deletions(-) diff --git a/engine/src/flutter/shell/platform/fuchsia/flutter/engine.cc b/engine/src/flutter/shell/platform/fuchsia/flutter/engine.cc index 3d8d53cddc6..3380726840a 100644 --- a/engine/src/flutter/shell/platform/fuchsia/flutter/engine.cc +++ b/engine/src/flutter/shell/platform/fuchsia/flutter/engine.cc @@ -128,10 +128,12 @@ Engine::Engine(Delegate& delegate, session_error_callback = std::move(session_error_callback), view_token = std::move(view_token), view_ref_pair = std::move(view_ref_pair), + max_frames_in_flight = product_config.get_max_frames_in_flight(), vsync_handle = vsync_event_.get()]() mutable { session_connection_.emplace( thread_label_, std::move(session), - std::move(session_error_callback), [](auto) {}, vsync_handle); + std::move(session_error_callback), [](auto) {}, vsync_handle, + max_frames_in_flight); surface_producer_.emplace(session_connection_->get()); #if defined(LEGACY_FUCHSIA_EMBEDDER) if (use_legacy_renderer_) { diff --git a/engine/src/flutter/shell/platform/fuchsia/flutter/flutter_runner_product_configuration.cc b/engine/src/flutter/shell/platform/fuchsia/flutter/flutter_runner_product_configuration.cc index 3406faa6c0c..c4f91e31925 100644 --- a/engine/src/flutter/shell/platform/fuchsia/flutter/flutter_runner_product_configuration.cc +++ b/engine/src/flutter/shell/platform/fuchsia/flutter/flutter_runner_product_configuration.cc @@ -20,6 +20,9 @@ FlutterRunnerProductConfiguration::FlutterRunnerProductConfiguration( if (auto& val = document["vsync_offset_in_us"]; val.IsInt()) { vsync_offset_ = fml::TimeDelta::FromMicroseconds(val.GetInt()); } + if (auto& val = document["max_frames_in_flight"]; val.IsInt()) { + max_frames_in_flight_ = val.GetInt(); + } #if defined(LEGACY_FUCHSIA_EMBEDDER) if (auto& val = document["use_legacy_renderer"]; val.IsBool()) { use_legacy_renderer_ = val.GetBool(); diff --git a/engine/src/flutter/shell/platform/fuchsia/flutter/flutter_runner_product_configuration.h b/engine/src/flutter/shell/platform/fuchsia/flutter/flutter_runner_product_configuration.h index 57313d7a58c..355f365042e 100644 --- a/engine/src/flutter/shell/platform/fuchsia/flutter/flutter_runner_product_configuration.h +++ b/engine/src/flutter/shell/platform/fuchsia/flutter/flutter_runner_product_configuration.h @@ -15,12 +15,14 @@ class FlutterRunnerProductConfiguration { FlutterRunnerProductConfiguration(std::string path); fml::TimeDelta get_vsync_offset() { return vsync_offset_; } + uint64_t get_max_frames_in_flight() { return max_frames_in_flight_; } #if defined(LEGACY_FUCHSIA_EMBEDDER) bool use_legacy_renderer() { return use_legacy_renderer_; } #endif private: fml::TimeDelta vsync_offset_ = fml::TimeDelta::Zero(); + uint64_t max_frames_in_flight_ = 3; #if defined(LEGACY_FUCHSIA_EMBEDDER) bool use_legacy_renderer_ = true; #endif diff --git a/engine/src/flutter/shell/platform/fuchsia/flutter/session_connection.cc b/engine/src/flutter/shell/platform/fuchsia/flutter/session_connection.cc index c87368a8993..c677d42fbc5 100644 --- a/engine/src/flutter/shell/platform/fuchsia/flutter/session_connection.cc +++ b/engine/src/flutter/shell/platform/fuchsia/flutter/session_connection.cc @@ -17,10 +17,12 @@ SessionConnection::SessionConnection( fidl::InterfaceHandle session, fml::closure session_error_callback, on_frame_presented_event on_frame_presented_callback, - zx_handle_t vsync_event_handle) + zx_handle_t vsync_event_handle, + uint64_t max_frames_in_flight) : session_wrapper_(session.Bind(), nullptr), on_frame_presented_callback_(std::move(on_frame_presented_callback)), - vsync_event_handle_(vsync_event_handle) { + vsync_event_handle_(vsync_event_handle), + kMaxFramesInFlight(max_frames_in_flight) { session_wrapper_.set_error_handler( [callback = session_error_callback](zx_status_t status) { callback(); }); diff --git a/engine/src/flutter/shell/platform/fuchsia/flutter/session_connection.h b/engine/src/flutter/shell/platform/fuchsia/flutter/session_connection.h index 7630d15e778..6f0eef073e5 100644 --- a/engine/src/flutter/shell/platform/fuchsia/flutter/session_connection.h +++ b/engine/src/flutter/shell/platform/fuchsia/flutter/session_connection.h @@ -27,7 +27,8 @@ class SessionConnection final : public flutter::SessionWrapper { fidl::InterfaceHandle session, fml::closure session_error_callback, on_frame_presented_event on_frame_presented_callback, - zx_handle_t vsync_event_handle); + zx_handle_t vsync_event_handle, + uint64_t max_frames_in_flight); ~SessionConnection(); @@ -71,7 +72,7 @@ class SessionConnection final : public flutter::SessionWrapper { // The maximum number of frames Flutter sent to Scenic that it can have // outstanding at any time. This is equivalent to how many times it has // called Present2() before receiving an OnFramePresented() event. - static constexpr int kMaxFramesInFlight = 3; + const int kMaxFramesInFlight; int frames_in_flight_ = 0; int frames_in_flight_allowed_ = 0; diff --git a/engine/src/flutter/shell/platform/fuchsia/flutter/tests/flutter_runner_product_configuration_unittests.cc b/engine/src/flutter/shell/platform/fuchsia/flutter/tests/flutter_runner_product_configuration_unittests.cc index 589228d4abe..1cdcd4a0a53 100644 --- a/engine/src/flutter/shell/platform/fuchsia/flutter/tests/flutter_runner_product_configuration_unittests.cc +++ b/engine/src/flutter/shell/platform/fuchsia/flutter/tests/flutter_runner_product_configuration_unittests.cc @@ -68,4 +68,24 @@ TEST_F(FlutterRunnerProductConfigurationTest, NonIntegerVsyncOffset) { EXPECT_EQ(product_config.get_vsync_offset(), expected_offset); } +TEST_F(FlutterRunnerProductConfigurationTest, ValidMaxFramesInFlight) { + const std::string json_string = "{ \"max_frames_in_flight\" : 5 } "; + const uint64_t expected_max_frames_in_flight = 5; + + FlutterRunnerProductConfiguration product_config = + FlutterRunnerProductConfiguration(json_string); + EXPECT_EQ(product_config.get_max_frames_in_flight(), + expected_max_frames_in_flight); +} + +TEST_F(FlutterRunnerProductConfigurationTest, MissingMaxFramesInFlight) { + const std::string json_string = "{ \"max_frames_in_flight\" : } "; + const uint64_t minimum_reasonable_max_frames_in_flight = 1; + + FlutterRunnerProductConfiguration product_config = + FlutterRunnerProductConfiguration(json_string); + EXPECT_GE(product_config.get_max_frames_in_flight(), + minimum_reasonable_max_frames_in_flight); +} + } // namespace flutter_runner_test diff --git a/engine/src/flutter/shell/platform/fuchsia/flutter/tests/session_connection_unittests.cc b/engine/src/flutter/shell/platform/fuchsia/flutter/tests/session_connection_unittests.cc index 5a5463266fa..b2c33a69313 100644 --- a/engine/src/flutter/shell/platform/fuchsia/flutter/tests/session_connection_unittests.cc +++ b/engine/src/flutter/shell/platform/fuchsia/flutter/tests/session_connection_unittests.cc @@ -70,7 +70,7 @@ TEST_F(SessionConnectionTest, SimplePresentTest) { flutter_runner::SessionConnection session_connection( "debug label", std::move(session_), on_session_error_callback, - on_frame_presented_callback, vsync_event_.get()); + on_frame_presented_callback, vsync_event_.get(), 3); for (int i = 0; i < 200; ++i) { session_connection.Present(); @@ -92,7 +92,7 @@ TEST_F(SessionConnectionTest, BatchedPresentTest) { flutter_runner::SessionConnection session_connection( "debug label", std::move(session_), on_session_error_callback, - on_frame_presented_callback, vsync_event_.get()); + on_frame_presented_callback, vsync_event_.get(), 3); for (int i = 0; i < 200; ++i) { session_connection.Present();