[flatland] Handle fence overflow in flatland_connection.cc

flatland_connection.cc used to allow an arbitrary number
of acquire and release fences to be scheduled for each frame.

Sadly, Fuchsia has a limitation of (1) the number of total handles
that can be sent per a FIDL call, but also (2) the Flatland protocol
only supports sending up to 16 fences per each fence type.

Now, normally there should be very few scheduled fences per frame. But
if frames get skipped, we could amass many fences which would then
crash our attempts to send all of them to the Flatland `Present`
endpoint.

This change introduces two fence multiplexer, which allow us to signal
more than 16 fences per type, at a performance penalty. We expect to
be able *not* to crash the FIDL subsystem using this approach, and may
even be able to hobble along for a bit, until the fences issue is
hopefully self-resolved.

That said, this issue seems to indicate there are frame scheduling
problems elsewhere. But this is a fairly straightforward change to make
without affecting the rest of the flatland code or integration, so we
opt to do that first.

Issue: #150136

shell/platform/fuchsia/flutter/flatland_connection.cc

@@ -4,10 +4,18 @@
 
 #include "flatland_connection.h"
 
+#include <lib/async/cpp/task.h>
+#include <lib/async/default.h>
+
+#include <zircon/rights.h>
 #include <zircon/status.h>
+#include <zircon/types.h>
+
+#include <utility>
 
 #include "flutter/fml/logging.h"
 #include "flutter/fml/trace_event.h"
+#include "fuchsia/ui/composition/cpp/fidl.h"
 
 namespace flutter_runner {
 
@@ -22,12 +30,14 @@ double DeltaFromNowInNanoseconds(const fml::TimePoint& now,
 }  // namespace
 
 FlatlandConnection::FlatlandConnection(
-    std::string debug_label,
+    const std::string& debug_label,
     fuchsia::ui::composition::FlatlandHandle flatland,
     fml::closure error_callback,
-    on_frame_presented_event on_frame_presented_callback)
-    : flatland_(flatland.Bind()),
-      error_callback_(error_callback),
+    on_frame_presented_event on_frame_presented_callback,
+    async_dispatcher_t* dispatcher)
+    : dispatcher_(dispatcher),
+      flatland_(flatland.Bind()),
+      error_callback_(std::move(error_callback)),
       on_frame_presented_callback_(std::move(on_frame_presented_callback)) {
   flatland_.set_error_handler([callback = error_callback_](zx_status_t status) {
     FML_LOG(ERROR) << "Flatland disconnected: " << zx_status_get_string(status);
@@ -52,6 +62,7 @@ void FlatlandConnection::Present() {
   if (threadsafe_state_.present_credits_ > 0) {
     DoPresent();
   } else {
+    FML_LOG(INFO) << "Wait for credits...";
     present_waiting_for_credit_ = true;
   }
 }
@@ -71,6 +82,42 @@ void FlatlandConnection::DoPresent() {
   fuchsia::ui::composition::PresentArgs present_args;
   present_args.set_requested_presentation_time(0);
   present_args.set_acquire_fences(std::move(acquire_fences_));
+
+  // Schedule acquire fence overflow signaling if there is one.
+  if (acquire_overflow_ != nullptr) {
+    FML_CHECK(acquire_overflow_->event.is_valid());
+    async::PostTask(dispatcher_, [dispatcher = dispatcher_,
+                                  overflow = acquire_overflow_]() {
+      const size_t fences_size = overflow->fences.size();
+      std::shared_ptr<size_t> fences_completed = std::make_shared<size_t>(0);
+      std::shared_ptr<std::vector<async::WaitOnce>> closures;
+
+      for (auto i = 0u; i < fences_size; i++) {
+        auto wait = std::make_unique<async::WaitOnce>(
+            overflow->fences[i].handle, ZX_EVENT_SIGNALED, 0u);
+        auto wait_ptr = wait.get();
+        wait_ptr->Begin(
+            dispatcher,
+            [wait = std::move(wait), overflow, fences_size, fences_completed,
+             closures](async_dispatcher_t*, async::WaitOnce*,
+                       zx_status_t status, const zx_packet_signal_t*) {
+              (*fences_completed)++;
+              FML_CHECK(status == ZX_OK)
+                  << "status: " << zx_status_get_string(status);
+              if (*fences_completed == fences_size) {
+                // Signal the acquire fence passed on to Flatland.
+                const zx_status_t status =
+                    overflow->event.signal(0, ZX_EVENT_SIGNALED);
+                FML_CHECK(status == ZX_OK)
+                    << "status: " << zx_status_get_string(status);
+              }
+            });
+      }
+    });
+    acquire_overflow_.reset();
+  }
+  FML_CHECK(acquire_overflow_ == nullptr);
+
   present_args.set_release_fences(std::move(previous_present_release_fences_));
   // Frame rate over latency.
   present_args.set_unsquashable(true);
@@ -81,6 +128,44 @@ void FlatlandConnection::DoPresent() {
   // the correct ones for VulkanSurface's interpretation.
   previous_present_release_fences_.clear();
   previous_present_release_fences_.swap(current_present_release_fences_);
+  previous_release_overflow_ = current_release_overflow_;
+  current_release_overflow_ = nullptr;
+
+  // Similar to the treatment of acquire_fences_overflow_ above. Except in
+  // the other direction.
+  if (previous_release_overflow_ != nullptr) {
+    FML_CHECK(previous_release_overflow_->event.is_valid());
+
+    std::shared_ptr<overflow_t<zx::event>> fences = previous_release_overflow_;
+
+    async::PostTask(dispatcher_, [dispatcher = dispatcher_,
+                                  fences = previous_release_overflow_]() {
+      FML_CHECK(fences != nullptr);
+      FML_CHECK(fences->event.is_valid());
+
+      auto wait = std::make_unique<async::WaitOnce>(fences->event.release(),
+                                                    ZX_EVENT_SIGNALED, 0u);
+      auto wait_ptr = wait.get();
+
+      wait_ptr->Begin(
+          dispatcher, [_wait = std::move(wait), fences](
+                          async_dispatcher_t*, async::WaitOnce*,
+                          zx_status_t status, const zx_packet_signal_t*) {
+            FML_CHECK(status == ZX_OK)
+                << "status: " << zx_status_get_string(status);
+
+            // Multiplex signaling all events.
+            for (auto& event : fences->fences) {
+              const zx_status_t status = event.signal(0, ZX_EVENT_SIGNALED);
+              FML_CHECK(status == ZX_OK)
+                  << "status: " << zx_status_get_string(status);
+            }
+          });
+    });
+    previous_release_overflow_ = nullptr;
+  }
+  FML_CHECK(previous_release_overflow_ == nullptr);  // Moved.
+
   acquire_fences_.clear();
 }
 
@@ -93,12 +178,13 @@ void FlatlandConnection::AwaitVsync(FireCallbackCallback callback) {
   const auto now = fml::TimePoint::Now();
 
   // Initial case.
-  if (MaybeRunInitialVsyncCallback(now, callback))
+  if (MaybeRunInitialVsyncCallback(now, callback)) {
     return;
+  }
 
   // Throttle case.
   if (threadsafe_state_.present_credits_ == 0) {
-    threadsafe_state_.pending_fire_callback_ = callback;
+    threadsafe_state_.pending_fire_callback_ = std::move(callback);
     return;
   }
 
@@ -116,8 +202,9 @@ void FlatlandConnection::AwaitVsyncForSecondaryCallback(
   const auto now = fml::TimePoint::Now();
 
   // Initial case.
-  if (MaybeRunInitialVsyncCallback(now, callback))
+  if (MaybeRunInitialVsyncCallback(now, callback)) {
     return;
+  }
 
   // Regular case.
   RunVsyncCallback(now, callback);
@@ -262,12 +349,106 @@ void FlatlandConnection::RunVsyncCallback(const fml::TimePoint& now,
 
 // This method is called from the raster thread.
 void FlatlandConnection::EnqueueAcquireFence(zx::event fence) {
-  acquire_fences_.push_back(std::move(fence));
+  constexpr size_t kMaxFences =
+      fuchsia::ui::composition::MAX_ACQUIRE_RELEASE_FENCE_COUNT;
+  // All prior fences, plus this one.
+  const auto total_size =
+      acquire_fences_.size() + 1 +
+      ((acquire_overflow_ == nullptr) ? 0 : acquire_overflow_->fences.size());
+
+  // If more than max number of fences come in, schedule any further fences into
+  // an overflow. The overflow data are scheduled here, but processed in
+  // DoPresent().
+
+  if (total_size <= kMaxFences) {
+    acquire_fences_.push_back(std::move(fence));
+  } else if (total_size == kMaxFences + 1) {
+    // The ownership of the overflow will be handed over to the signaling
+    // closure on DoPresent call. So we always expect that we enter here with
+    // overflow not set.
+    FML_CHECK(acquire_overflow_ == nullptr)
+        << "acquire_overflow_ is still active";
+    acquire_overflow_ = std::make_shared<overflow_t<zx_wait_item_t>>();
+
+    // Set up the overflow fences. Make a new fence, put the last
+    // fence and the new fence into overflow.
+    zx::event overflow_handle = std::move(acquire_fences_.back());
+    acquire_fences_.pop_back();
+
+    zx::event overflow_fence;
+    const zx_status_t status = zx::event::create(0, &overflow_fence);
+    FML_CHECK(status == ZX_OK) << "status: " << zx_status_get_string(status);
+
+    // Every DoPresent should invalidate this handle.  Holler if not.
+    FML_CHECK(!acquire_overflow_->event.is_valid())
+        << "acquire overflow event is still valid";
+    const zx_status_t status2 = overflow_fence.duplicate(
+        ZX_RIGHT_SAME_RIGHTS, &acquire_overflow_->event);
+    acquire_fences_.push_back(std::move(overflow_fence));
+
+    // Prepare for wait_many call.
+    acquire_overflow_->fences.push_back(zx_wait_item_t{
+        .handle = std::move(overflow_handle).release(),
+        .waitfor = ZX_EVENT_SIGNALED,
+        .pending = 0u,
+    });
+    acquire_overflow_->fences.push_back(zx_wait_item_t{
+        .handle = std::move(fence).release(),
+        .waitfor = ZX_EVENT_SIGNALED,
+        .pending = 0u,
+    });
+
+    FML_LOG(INFO) << "FlatlandConnection::EnqueueAcquireFence: "
+                  << "using fence overflow, expect a performance hit.";
+  } else {
+    FML_CHECK(acquire_overflow_ != nullptr);
+    // Just add to the overflow fences.
+    acquire_overflow_->fences.push_back(zx_wait_item_t{
+        .handle = std::move(fence).release(),
+        .waitfor = ZX_EVENT_SIGNALED,
+        .pending = 0u,
+    });
+  }
 }
 
 // This method is called from the raster thread.
 void FlatlandConnection::EnqueueReleaseFence(zx::event fence) {
-  current_present_release_fences_.push_back(std::move(fence));
+  // The approach here is similar to above. See the comments there for
+  // details
+  const auto total_size = current_present_release_fences_.size() + 1 +
+                          ((current_release_overflow_ == nullptr)
+                               ? 0
+                               : current_release_overflow_->fences.size());
+  if (total_size <= kMaxFences) {
+    current_present_release_fences_.push_back(std::move(fence));
+  } else if (total_size == kMaxFences + 1) {
+    FML_CHECK(current_release_overflow_ == nullptr);
+
+    current_release_overflow_ = std::make_shared<overflow_t<zx::event>>();
+    zx::event overflow_handle =
+        std::move(current_present_release_fences_.back());
+    current_present_release_fences_.pop_back();
+    zx::event overflow_fence;
+    const zx_status_t status = zx::event::create(0, &overflow_fence);
+    FML_CHECK(status == ZX_OK) << "status: " << zx_status_get_string(status);
+    FML_CHECK(!current_release_overflow_->event.is_valid());
+
+    const zx_status_t status2 = overflow_fence.duplicate(
+        ZX_RIGHT_SAME_RIGHTS, &current_release_overflow_->event);
+    FML_CHECK(status2 == ZX_OK) << "status: " << zx_status_get_string(status);
+
+    current_present_release_fences_.push_back(std::move(overflow_fence));
+    current_release_overflow_->fences.push_back(std::move(overflow_handle));
+    current_release_overflow_->fences.push_back(std::move(fence));
+
+    FML_LOG(INFO) << "FlatlandConnection::EnqueueReleaseFence: "
+                  << "using fence overflow, expect a performance hit.";
+  } else {
+    FML_CHECK(current_release_overflow_ != nullptr);
+
+    // Just add to the overflow fences.
+    current_release_overflow_->fences.push_back(std::move(fence));
+  }
 }
 
 }  // namespace flutter_runner

shell/platform/fuchsia/flutter/flatland_connection.h

@@ -2,10 +2,14 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+// Maintains a connection to Fuchsia's Flatland protocol used for rendering
+// 2D graphics scenes.
+
 #ifndef FLUTTER_SHELL_PLATFORM_FUCHSIA_FLUTTER_FLATLAND_CONNECTION_H_
 #define FLUTTER_SHELL_PLATFORM_FUCHSIA_FLUTTER_FLATLAND_CONNECTION_H_
 
 #include <fuchsia/ui/composition/cpp/fidl.h>
+#include <lib/async/default.h>
 
 #include "flutter/fml/closure.h"
 #include "flutter/fml/macros.h"
@@ -33,10 +37,12 @@ static constexpr fml::TimeDelta kInitialFlatlandVsyncOffset =
 // maintaining the Flatland instance connection and presenting updates.
 class FlatlandConnection final {
  public:
-  FlatlandConnection(std::string debug_label,
-                     fuchsia::ui::composition::FlatlandHandle flatland,
-                     fml::closure error_callback,
-                     on_frame_presented_event on_frame_presented_callback);
+  FlatlandConnection(
+      const std::string& debug_label,
+      fuchsia::ui::composition::FlatlandHandle flatland,
+      fml::closure error_callback,
+      on_frame_presented_event on_frame_presented_callback,
+      async_dispatcher_t* dispatcher = async_get_default_dispatcher());
 
   ~FlatlandConnection();
 
@@ -58,7 +64,24 @@ class FlatlandConnection final {
     return {++next_content_id_};
   }
 
+  // Adds a new acquire fence to be sent out to the next Present() call.
+  //
+  // Acquire fences must all be signaled by the user.
+  //
+  // PERFORMANCE NOTES:
+  //
+  // Enqueuing more than 16 fences per frame incurs a performance penalty, so
+  // use them sparingly.
+  //
+  // Skipped frames may cause the number of fences to increase, leading to
+  // more performance issues. Ideally, the flow of the frames should be smooth.
   void EnqueueAcquireFence(zx::event fence);
+
+  // Adds a new release fence to be sent out to the next Present() call.
+  //
+  // Release fences are all signaled by Scenic (Flatland server).
+  //
+  // See the performance notes on EnqueueAcquireFence for performance details.
   void EnqueueReleaseFence(zx::event fence);
 
  private:
@@ -75,6 +98,8 @@ class FlatlandConnection final {
   void RunVsyncCallback(const fml::TimePoint& now,
                         FireCallbackCallback& callback);
 
+  async_dispatcher_t* dispatcher_;
+
   fuchsia::ui::composition::FlatlandPtr flatland_;
 
   fml::closure error_callback_;
@@ -102,9 +127,39 @@ class FlatlandConnection final {
     bool first_feedback_received_ = false;
   } threadsafe_state_;
 
+  // The maximum number of fences that can be signaled at a time.
+  static constexpr size_t kMaxFences =
+      fuchsia::ui::composition::MAX_ACQUIRE_RELEASE_FENCE_COUNT;
+
+  // A helper to ferry around multiplexed events for signaling.
+  //
+  // Used either to wait on event, then signal fences, or to
+  // wait on many fences, then signal event.
+  template <typename Q>
+  class overflow_t {
+   public:
+    // Usually not copyable.
+    std::vector<Q> fences;
+    // Definitely not copyable.
+    zx::event event{};
+    overflow_t() { fences.reserve(kMaxFences); };
+  };
+
+  // Acquire fences sent to Flatland.
   std::vector<zx::event> acquire_fences_;
+
+  // Multiplexed acquire fences over the critical number of ~16.
+  std::shared_ptr<overflow_t<zx_wait_item_t>> acquire_overflow_;
+
+  // Release fences sent to Flatland. Similar to acquire fences above.
   std::vector<zx::event> current_present_release_fences_;
+  std::shared_ptr<overflow_t<zx::event>> current_release_overflow_;
+
+  // Release fences from the prior call to DoPresent().
+  // Similar to acquire fences above.
   std::vector<zx::event> previous_present_release_fences_;
+  std::shared_ptr<overflow_t<zx::event>> previous_release_overflow_;
+
   std::string debug_label_;
 
   FML_DISALLOW_COPY_AND_ASSIGN(FlatlandConnection);

shell/platform/fuchsia/flutter/tests/fakes/scenic/fake_flatland.cc

@@ -41,7 +41,7 @@ fuchsia::ui::composition::FlatlandHandle FakeFlatland::ConnectFlatland(
 }
 
 void FakeFlatland::Disconnect(fuchsia::ui::composition::FlatlandError error) {
-  flatland_binding_.events().OnError(std::move(error));
+  flatland_binding_.events().OnError(error);
   flatland_binding_.Unbind();
   allocator_binding_
       .Unbind();  // TODO(fxb/85619): Does the real Scenic unbind this when