Tidy the bridgeless runtime executor plumbing

Summary:
Follow-ups to the buffered CallInvoker, kept out of that diff so the behavioural
change stays reviewable on its own. No behaviour change intended here.

- `BufferedRuntimeExecutor` gains `asRuntimeExecutor()` and
  `asWeakRuntimeExecutor()`. RuntimeExecutor is a std::function, so callers
  cannot hand it the object and instead each hand-rolled the same capturing
  lambda; the ownership question now has one answer per lifetime policy. The
  weak form is what `getBufferedRuntimeExecutor` needs, which must not keep the
  instance alive.
- `BufferedWork` becomes private. It was only ever an implementation detail, and
  being public put its fields in the C++ API snapshot.
- The buffer holds a `std::vector` sorted once at flush rather than a
  `std::priority_queue`. Indices are handed out before the lock, so arrival
  order can differ from submission order and something has to restore it — but a
  heap is the expensive way. `std::priority_queue::top()` returns a const
  reference, so every flushed item copied its `std::function` on the way out;
  that copy is now a move, and nothing sifts on the way in.
- `runtimeExecutorThatGoesThroughRuntimeScheduler` was a second copy of
  `getUnbufferedRuntimeExecutor()`; it now calls it.
- `RuntimeSchedulerCallInvoker` is marked deprecated. Its remaining users are
  migrated: `ReactCxxPlatform` holds a `ReactInstance` and can use
  `createJSCallInvoker()`, and four other files only `#include`d it without ever
  constructing one. The single remaining use is the flag-off branch, which is
  suppressed locally and goes when the flag is cleaned up.
- Drops a null check on `bufferedRuntimeExecutor_` in `callFunctionOnModule`
  that the constructor makes unreachable.

Changelog:
[Internal]

Differential Revision: D118616045

Author: javache

packages/react-native/ReactCommon/react/runtime/BufferedRuntimeExecutor.cpp

@@ -7,17 +7,16 @@
 
 #include "BufferedRuntimeExecutor.h"
 
+#include <algorithm>
+#include <utility>
+
 namespace facebook::react {
 
 BufferedRuntimeExecutor::BufferedRuntimeExecutor(Executor executor)
     : executor_(std::move(executor)),
       isBufferingEnabled_(true),
       lastIndex_(0) {}
 
-void BufferedRuntimeExecutor::execute(Work&& callback) {
-  execute(SchedulerPriority::ImmediatePriority, std::move(callback));
-}
-
 void BufferedRuntimeExecutor::execute(
     SchedulerPriority priority,
     Work&& callback) {
@@ -35,7 +34,7 @@ void BufferedRuntimeExecutor::execute(
   uint64_t newIndex = lastIndex_++;
   std::scoped_lock guard(lock_);
   if (isBufferingEnabled_) {
-    queue_.push(
+    queue_.push_back(
         {.index_ = newIndex,
          .work_ = std::move(callback),
          .priority_ = priority});
@@ -48,18 +47,41 @@ void BufferedRuntimeExecutor::execute(
   executor_(priority, std::move(callback));
 }
 
+RuntimeExecutor BufferedRuntimeExecutor::asRuntimeExecutor() {
+  return [self = shared_from_this()](Work&& callback) {
+    self->execute(SchedulerPriority::ImmediatePriority, std::move(callback));
+  };
+}
+
+RuntimeExecutor BufferedRuntimeExecutor::asWeakRuntimeExecutor() {
+  return [weakSelf = weak_from_this()](Work&& callback) {
+    if (auto self = weakSelf.lock()) {
+      self->execute(SchedulerPriority::ImmediatePriority, std::move(callback));
+    }
+  };
+}
+
 void BufferedRuntimeExecutor::flush() {
   std::scoped_lock guard(lock_);
   unsafeFlush();
   isBufferingEnabled_ = false;
 }
 
 void BufferedRuntimeExecutor::unsafeFlush() {
-  while (!queue_.empty()) {
-    const BufferedWork& bufferedWork = queue_.top();
-    Work work = bufferedWork.work_;
-    executor_(bufferedWork.priority_, std::move(work));
-    queue_.pop();
+  // Indices are handed out before the lock is taken, so arrival order can
+  // differ from submission order. Sorting once here restores it, and costs less
+  // than a heap did: nothing sifts on the way in, and each callback is moved
+  // out rather than copied.
+  auto batch = std::move(queue_);
+  queue_.clear();
+  std::sort(
+      batch.begin(),
+      batch.end(),
+      [](const BufferedWork& lhs, const BufferedWork& rhs) {
+        return lhs.index_ < rhs.index_;
+      });
+  for (auto& bufferedWork : batch) {
+    executor_(bufferedWork.priority_, std::move(bufferedWork.work_));
   }
 }
 

packages/react-native/ReactCommon/react/runtime/BufferedRuntimeExecutor.h

@@ -11,39 +11,25 @@
 #include <ReactCommon/SchedulerPriority.h>
 #include <jsi/jsi.h>
 #include <atomic>
+#include <memory>
 #include <mutex>
-#include <queue>
+#include <vector>
 
 namespace facebook::react {
 
-class BufferedRuntimeExecutor {
+class BufferedRuntimeExecutor : public std::enable_shared_from_this<BufferedRuntimeExecutor> {
  public:
   using Work = std::function<void(jsi::Runtime &runtime)>;
 
-  /**
-   * Drains one piece of buffered work. Always given a priority; an executor
-   * that sits below the RuntimeScheduler, and so has no notion of one, ignores
-   * it.
-   */
+  // Drains one piece of buffered work, priority may be ignored
   using Executor = std::function<void(SchedulerPriority, Work &&)>;
 
-  // A utility structure to track pending work in the order of when they arrive.
-  struct BufferedWork {
-    uint64_t index_;
-    Work work_;
-    SchedulerPriority priority_;
-    bool operator<(const BufferedWork &rhs) const
-    {
-      // Higher index has lower priority, so this inverted comparison puts
-      // the smaller index on top of the queue.
-      return index_ > rhs.index_;
-    }
-  };
-
   BufferedRuntimeExecutor(Executor executor);
 
-  /** Equivalent to `execute(SchedulerPriority::ImmediatePriority, ...)`. */
-  void execute(Work &&callback);
+  void execute(Work &&callback)
+  {
+    execute(SchedulerPriority::ImmediatePriority, std::move(callback));
+  }
 
   /**
    * Buffers [callback] alongside work submitted through the other overload,
@@ -52,18 +38,37 @@ class BufferedRuntimeExecutor {
    */
   void execute(SchedulerPriority priority, Work &&callback);
 
+  /**
+   * RuntimeExecutor, keeping this class alive for as long as the result is
+   * held.
+   */
+  RuntimeExecutor asRuntimeExecutor();
+
+  /**
+   * RuntimeExecutor, keeping a weak reference to this class, so it does not
+   * keep the runtime alive unnecessarily.
+   */
+  RuntimeExecutor asWeakRuntimeExecutor();
+
   // Flush buffered JS calls and then diable JS buffering
   void flush();
 
  private:
   // Perform flushing without locking mechanism
   void unsafeFlush();
 
+  // A utility structure to track pending work in the order of when they arrive.
+  struct BufferedWork {
+    uint64_t index_;
+    Work work_;
+    SchedulerPriority priority_;
+  };
+
   Executor executor_;
   std::atomic<bool> isBufferingEnabled_;
   std::mutex lock_;
   std::atomic<uint64_t> lastIndex_;
-  std::priority_queue<BufferedWork> queue_;
+  std::vector<BufferedWork> queue_;
 };
 
 } // namespace facebook::react

packages/react-native/ReactCommon/react/runtime/ReactInstance.cpp

@@ -124,32 +124,18 @@ ReactInstance::ReactInstance(
                 std::function<void(jsi::Runtime & runtime)>&& callback) {
               runtimeExecutor(std::move(callback));
             });
-    auto runtimeExecutorThatExecutesAfterInspectorSetup =
-        [bufferedRuntimeExecutorThatWaitsForInspectorSetup](
-            std::function<void(jsi::Runtime & runtime)>&& callback) {
-          bufferedRuntimeExecutorThatWaitsForInspectorSetup->execute(
-              std::move(callback));
-        };
 
     runtimeScheduler_ = createRuntimeScheduler(
-        runtimeExecutorThatExecutesAfterInspectorSetup,
+        bufferedRuntimeExecutorThatWaitsForInspectorSetup->asRuntimeExecutor(),
         [jsErrorHandler = jsErrorHandler_](
             jsi::Runtime& runtime, jsi::JSError& error) {
           jsErrorHandler->handleError(runtime, error, true);
         });
 
-    auto runtimeExecutorThatGoesThroughRuntimeScheduler =
-        [runtimeScheduler = runtimeScheduler_.get()](
-            std::function<void(jsi::Runtime & runtime)>&& callback) {
-          runtimeScheduler->scheduleWork(std::move(callback));
-        };
-
     // This code can execute from any thread, so we need to make sure we set up
     // the inspector logic in the right one. The callback executes immediately
     // if we are already in the right thread.
-    executor([this,
-              runtimeExecutorThatGoesThroughRuntimeScheduler,
-              bufferedRuntimeExecutorThatWaitsForInspectorSetup](
+    executor([this, bufferedRuntimeExecutorThatWaitsForInspectorSetup](
                  jsinspector_modern::HostTarget& hostTarget) {
       // Callbacks scheduled through the page target executor are generally
       // not guaranteed to run (e.g.: if the page target is destroyed)
@@ -160,8 +146,7 @@ ReactInstance::ReactInstance(
       //   creation task to finish before starting the destruction.
       inspectorTarget_ = &hostTarget.registerInstance(*this);
       runtimeInspectorTarget_ = &inspectorTarget_->registerRuntime(
-          runtime_->getRuntimeTargetDelegate(),
-          runtimeExecutorThatGoesThroughRuntimeScheduler);
+          runtime_->getRuntimeTargetDelegate(), getUnbufferedRuntimeExecutor());
       bufferedRuntimeExecutorThatWaitsForInspectorSetup->flush();
     });
   } else {
@@ -178,9 +163,9 @@ ReactInstance::ReactInstance(
         setHermesEventLoopControl(runtime, runtimeScheduler);
       });
 
-  // `scheduleWork` is `scheduleTask(ImmediatePriority)` on the modern
-  // scheduler, which is the only one bridgeless uses, so routing everything
-  // through `scheduleTask` leaves unprioritised callers where they were.
+  // Note that bufferedRuntimeExecutor_ only has a raw pointer to
+  // RuntimeScheduler It should always be retained weakly, as it should be
+  // destroyed when the runtime is.
   bufferedRuntimeExecutor_ = std::make_shared<BufferedRuntimeExecutor>(
       [runtimeScheduler = runtimeScheduler_.get()](
           SchedulerPriority priority,
@@ -214,7 +199,8 @@ void ReactInstance::unregisterFromInspector() {
 RuntimeExecutor ReactInstance::getUnbufferedRuntimeExecutor() noexcept {
   return [runtimeScheduler = runtimeScheduler_.get()](
              std::function<void(jsi::Runtime & runtime)>&& callback) {
-    runtimeScheduler->scheduleWork(std::move(callback));
+    runtimeScheduler->scheduleTask(
+        SchedulerPriority::ImmediatePriority, std::move(callback));
   };
 }
 
@@ -223,14 +209,7 @@ RuntimeExecutor ReactInstance::getUnbufferedRuntimeExecutor() noexcept {
 // getUnbufferedRuntimeExecutor() instead if you do not need the main JS
 // bundle to have finished. e.g. setting global variables into JS runtime.
 RuntimeExecutor ReactInstance::getBufferedRuntimeExecutor() noexcept {
-  return [weakBufferedRuntimeExecutor_ =
-              std::weak_ptr<BufferedRuntimeExecutor>(bufferedRuntimeExecutor_)](
-             std::function<void(jsi::Runtime & runtime)>&& callback) {
-    if (auto strongBufferedRuntimeExecutor_ =
-            weakBufferedRuntimeExecutor_.lock()) {
-      strongBufferedRuntimeExecutor_->execute(std::move(callback));
-    }
-  };
+  return bufferedRuntimeExecutor_->asWeakRuntimeExecutor();
 }
 
 // TODO(T184010230): Should the RuntimeScheduler returned from this method be

scripts/cxx-api/api-snapshots/ReactAndroidDebugCxx.api

@@ -1901,22 +1901,17 @@ class facebook::react::BridgelessNativeMethodCallInvoker : public facebook::reac
   public virtual void invokeSync(const std::string& methodName, facebook::react::NativeMethodCallFunc&& func) override;
 }
 
-class facebook::react::BufferedRuntimeExecutor {
+class facebook::react::BufferedRuntimeExecutor : public std::enable_shared_from_this<facebook::react::BufferedRuntimeExecutor> {
   public BufferedRuntimeExecutor(facebook::react::BufferedRuntimeExecutor::Executor executor);
+  public facebook::react::RuntimeExecutor asRuntimeExecutor();
+  public facebook::react::RuntimeExecutor asWeakRuntimeExecutor();
   public using Executor = std::function<void(facebook::react::SchedulerPriority, facebook::react::BufferedRuntimeExecutor::Work&&)>;
   public using Work = std::function<void(facebook::jsi::Runtime& runtime)>;
   public void execute(facebook::react::BufferedRuntimeExecutor::Work&& callback);
   public void execute(facebook::react::SchedulerPriority priority, facebook::react::BufferedRuntimeExecutor::Work&& callback);
   public void flush();
 }
 
-struct facebook::react::BufferedRuntimeExecutor::BufferedWork {
-  public bool operator<(const facebook::react::BufferedRuntimeExecutor::BufferedWork& rhs) const;
-  public facebook::react::BufferedRuntimeExecutor::Work work_;
-  public facebook::react::SchedulerPriority priority_;
-  public uint64_t index_;
-}
-
 class facebook::react::CSSSyntaxParser {
   public constexpr CSSSyntaxParser(const facebook::react::CSSSyntaxParser&) = default;
   public constexpr CSSSyntaxParser(facebook::react::CSSSyntaxParser&&) = default;

scripts/cxx-api/api-snapshots/ReactAndroidNewarchCxx.api

@@ -1895,22 +1895,17 @@ class facebook::react::BridgelessNativeMethodCallInvoker : public facebook::reac
   public virtual void invokeSync(const std::string& methodName, facebook::react::NativeMethodCallFunc&& func) override;
 }
 
-class facebook::react::BufferedRuntimeExecutor {
+class facebook::react::BufferedRuntimeExecutor : public std::enable_shared_from_this<facebook::react::BufferedRuntimeExecutor> {
   public BufferedRuntimeExecutor(facebook::react::BufferedRuntimeExecutor::Executor executor);
+  public facebook::react::RuntimeExecutor asRuntimeExecutor();
+  public facebook::react::RuntimeExecutor asWeakRuntimeExecutor();
   public using Executor = std::function<void(facebook::react::SchedulerPriority, facebook::react::BufferedRuntimeExecutor::Work&&)>;
   public using Work = std::function<void(facebook::jsi::Runtime& runtime)>;
   public void execute(facebook::react::BufferedRuntimeExecutor::Work&& callback);
   public void execute(facebook::react::SchedulerPriority priority, facebook::react::BufferedRuntimeExecutor::Work&& callback);
   public void flush();
 }
 
-struct facebook::react::BufferedRuntimeExecutor::BufferedWork {
-  public bool operator<(const facebook::react::BufferedRuntimeExecutor::BufferedWork& rhs) const;
-  public facebook::react::BufferedRuntimeExecutor::Work work_;
-  public facebook::react::SchedulerPriority priority_;
-  public uint64_t index_;
-}
-
 class facebook::react::CSSSyntaxParser {
   public constexpr CSSSyntaxParser(const facebook::react::CSSSyntaxParser&) = default;
   public constexpr CSSSyntaxParser(facebook::react::CSSSyntaxParser&&) = default;

scripts/cxx-api/api-snapshots/ReactAndroidReleaseCxx.api

@@ -1899,22 +1899,17 @@ class facebook::react::BridgelessNativeMethodCallInvoker : public facebook::reac
   public virtual void invokeSync(const std::string& methodName, facebook::react::NativeMethodCallFunc&& func) override;
 }
 
-class facebook::react::BufferedRuntimeExecutor {
+class facebook::react::BufferedRuntimeExecutor : public std::enable_shared_from_this<facebook::react::BufferedRuntimeExecutor> {
   public BufferedRuntimeExecutor(facebook::react::BufferedRuntimeExecutor::Executor executor);
+  public facebook::react::RuntimeExecutor asRuntimeExecutor();
+  public facebook::react::RuntimeExecutor asWeakRuntimeExecutor();
   public using Executor = std::function<void(facebook::react::SchedulerPriority, facebook::react::BufferedRuntimeExecutor::Work&&)>;
   public using Work = std::function<void(facebook::jsi::Runtime& runtime)>;
   public void execute(facebook::react::BufferedRuntimeExecutor::Work&& callback);
   public void execute(facebook::react::SchedulerPriority priority, facebook::react::BufferedRuntimeExecutor::Work&& callback);
   public void flush();
 }
 
-struct facebook::react::BufferedRuntimeExecutor::BufferedWork {
-  public bool operator<(const facebook::react::BufferedRuntimeExecutor::BufferedWork& rhs) const;
-  public facebook::react::BufferedRuntimeExecutor::Work work_;
-  public facebook::react::SchedulerPriority priority_;
-  public uint64_t index_;
-}
-
 class facebook::react::CSSSyntaxParser {
   public constexpr CSSSyntaxParser(const facebook::react::CSSSyntaxParser&) = default;
   public constexpr CSSSyntaxParser(facebook::react::CSSSyntaxParser&&) = default;

scripts/cxx-api/api-snapshots/ReactAppleDebugCxx.api

@@ -4484,22 +4484,17 @@ class facebook::react::BridgelessNativeMethodCallInvoker : public facebook::reac
   public virtual void invokeSync(const std::string& methodName, facebook::react::NativeMethodCallFunc&& func) override;
 }
 
-class facebook::react::BufferedRuntimeExecutor {
+class facebook::react::BufferedRuntimeExecutor : public std::enable_shared_from_this<facebook::react::BufferedRuntimeExecutor> {
   public BufferedRuntimeExecutor(facebook::react::BufferedRuntimeExecutor::Executor executor);
+  public facebook::react::RuntimeExecutor asRuntimeExecutor();
+  public facebook::react::RuntimeExecutor asWeakRuntimeExecutor();
   public using Executor = std::function<void(facebook::react::SchedulerPriority, facebook::react::BufferedRuntimeExecutor::Work&&)>;
   public using Work = std::function<void(facebook::jsi::Runtime& runtime)>;
   public void execute(facebook::react::BufferedRuntimeExecutor::Work&& callback);
   public void execute(facebook::react::SchedulerPriority priority, facebook::react::BufferedRuntimeExecutor::Work&& callback);
   public void flush();
 }
 
-struct facebook::react::BufferedRuntimeExecutor::BufferedWork {
-  public bool operator<(const facebook::react::BufferedRuntimeExecutor::BufferedWork& rhs) const;
-  public facebook::react::BufferedRuntimeExecutor::Work work_;
-  public facebook::react::SchedulerPriority priority_;
-  public uint64_t index_;
-}
-
 class facebook::react::CSSSyntaxParser {
   public constexpr CSSSyntaxParser(const facebook::react::CSSSyntaxParser&) = default;
   public constexpr CSSSyntaxParser(facebook::react::CSSSyntaxParser&&) = default;

scripts/cxx-api/api-snapshots/ReactAppleNewarchCxx.api

@@ -4471,22 +4471,17 @@ class facebook::react::BridgelessNativeMethodCallInvoker : public facebook::reac
   public virtual void invokeSync(const std::string& methodName, facebook::react::NativeMethodCallFunc&& func) override;
 }
 
-class facebook::react::BufferedRuntimeExecutor {
+class facebook::react::BufferedRuntimeExecutor : public std::enable_shared_from_this<facebook::react::BufferedRuntimeExecutor> {
   public BufferedRuntimeExecutor(facebook::react::BufferedRuntimeExecutor::Executor executor);
+  public facebook::react::RuntimeExecutor asRuntimeExecutor();
+  public facebook::react::RuntimeExecutor asWeakRuntimeExecutor();
   public using Executor = std::function<void(facebook::react::SchedulerPriority, facebook::react::BufferedRuntimeExecutor::Work&&)>;
   public using Work = std::function<void(facebook::jsi::Runtime& runtime)>;
   public void execute(facebook::react::BufferedRuntimeExecutor::Work&& callback);
   public void execute(facebook::react::SchedulerPriority priority, facebook::react::BufferedRuntimeExecutor::Work&& callback);
   public void flush();
 }
 
-struct facebook::react::BufferedRuntimeExecutor::BufferedWork {
-  public bool operator<(const facebook::react::BufferedRuntimeExecutor::BufferedWork& rhs) const;
-  public facebook::react::BufferedRuntimeExecutor::Work work_;
-  public facebook::react::SchedulerPriority priority_;
-  public uint64_t index_;
-}
-
 class facebook::react::CSSSyntaxParser {
   public constexpr CSSSyntaxParser(const facebook::react::CSSSyntaxParser&) = default;
   public constexpr CSSSyntaxParser(facebook::react::CSSSyntaxParser&&) = default;