From 5cb5b9ac2fef31f458d69c74dc04282ff90e04e0 Mon Sep 17 00:00:00 2001 From: Peter Abbondanzo Date: Sun, 11 Oct 2026 15:18:15 -0700 Subject: [PATCH] Bound the display-phase event beat induce to one per run loop turn (#58642) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Summary: The beat an Apple display phase induces runs the whole event loop tick on the calling thread, mounting included. A view whose layout metrics change during that mount can emit another synchronous request from inside the display that is servicing the first one, and Core Animation honours a `setNeedsDisplay` made during a display by running the commit's layout and display phases again, with no bound. One Core Animation commit can therefore perform an unbounded number of blocking JavaScript round trips. `AppleEventBeat` now induces at most once per run loop turn from the display phase. The run loop observer, which runs before Core Animation's commit observer, opens each turn. Requests arriving after that first induce keep the ordinary observer timing — what they had before the display-phase induce existed — so this bounds the tail without giving up the guarantee the display-phase induce was added for: the first layout-driven request of a frame is still processed in that frame. Measured on an iPhone 11 / iOS 26.0 simulator with a virtualized list whose placeholder is taller than its content, so making one row visible pulls siblings into the viewport. Identical scenario, 13 synchronous requests in every arm: | arm | worst single main-thread block | induces in one commit | | --- | --- | --- | | display-phase induce, unbounded | 145.8 ms | 11 | | no display-phase induce | 15.5 ms | n/a | | display-phase induce, bounded (this change) | 14.9 ms | 1 | A separate probe established that Core Animation itself imposes no bound: a zero-sized layer that re-dirties itself from inside its own `display` ran 200 display passes in one commit, stopped only by the probe's own cap. Changelog: [iOS][Fixed] - Process at most one synchronous event beat per Core Animation commit, so a mount performed during the display phase cannot re-enter it without bound Reviewed By: andrewdacenko, javache Differential Revision: D120951592 --- .../React/Fabric/AppleEventBeat.h | 14 + .../React/Fabric/AppleEventBeat.mm | 7 + .../React/Tests/Fabric/AppleEventBeatTests.mm | 241 ++++++++++++++++++ 3 files changed, 262 insertions(+) create mode 100644 packages/react-native/React/Tests/Fabric/AppleEventBeatTests.mm diff --git a/packages/react-native/React/Fabric/AppleEventBeat.h b/packages/react-native/React/Fabric/AppleEventBeat.h index 71ea0a112dcd..4a11927bafb6 100644 --- a/packages/react-native/React/Fabric/AppleEventBeat.h +++ b/packages/react-native/React/Fabric/AppleEventBeat.h @@ -70,6 +70,20 @@ class AppleEventBeat : public EventBeat, public RunLoopObserver::Delegate { WindowLayerResolver windowLayerResolver_; NSMapTable *layers_; void (^onDisplay_)(void); + + /* + * Whether a display phase has already induced in the current run loop turn. + * + * The beat a display phase induces runs the whole event loop tick on the + * main thread, mounting included, so a `VirtualView` whose layout metrics + * change during that mount emits another synchronous request from inside the + * display it is servicing. Core Animation honours `setNeedsDisplay` made + * during a display by running the commit's layout and display phases again, + * with no bound, so without this flag one commit can perform an unbounded + * number of blocking JavaScript round trips. Requests that arrive after the + * first induce keep the ordinary run loop observer timing. + */ + mutable bool didInduceInCurrentTurn_{false}; }; } // namespace facebook::react diff --git a/packages/react-native/React/Fabric/AppleEventBeat.mm b/packages/react-native/React/Fabric/AppleEventBeat.mm index 313231919627..2a4cb1588e75 100644 --- a/packages/react-native/React/Fabric/AppleEventBeat.mm +++ b/packages/react-native/React/Fabric/AppleEventBeat.mm @@ -69,6 +69,10 @@ - (void)display if (!owner) { return; } + if (this->didInduceInCurrentTurn_) { + return; + } + this->didInduceInCurrentTurn_ = true; this->induce(); }; @@ -117,6 +121,9 @@ - (void)display RunLoopObserver::Activity /*activity*/) const noexcept { react_native_assert(delegate == this); + // This observer runs before Core Animation's commit observer, so it is the + // start of the turn whose display phase `didInduceInCurrentTurn_` bounds. + didInduceInCurrentTurn_ = false; induce(); } diff --git a/packages/react-native/React/Tests/Fabric/AppleEventBeatTests.mm b/packages/react-native/React/Tests/Fabric/AppleEventBeatTests.mm new file mode 100644 index 000000000000..84eed211f204 --- /dev/null +++ b/packages/react-native/React/Tests/Fabric/AppleEventBeatTests.mm @@ -0,0 +1,241 @@ +/* + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +#import +#import + +#import +#import +#import + +#import + +#import +#import +#import +#import +#import + +// `AppleEventBeat.h` is private to `RCTFabric` and is not reachable from a +// dependent target, so the test target declares it as its own header rather +// than the library exporting it for one test's sake. +#import "AppleEventBeat.h" + +using facebook::react::AppleEventBeat; +using facebook::react::EventBeat; +using facebook::react::kNoTag; +using facebook::react::ReactNativeFeatureFlags; +using facebook::react::ReactNativeFeatureFlagsDefaults; +using facebook::react::RunLoopObserver; +using facebook::react::RuntimeExecutor; +using facebook::react::RuntimeScheduler; +using facebook::react::Tag; + +namespace { + +/* + * The beat only needs an observer it can own; the tests drive the turn + * boundary through `activityDidChange` directly, which is what the real + * observer does. + */ +class StubRunLoopObserver : public RunLoopObserver { + public: + using RunLoopObserver::RunLoopObserver; + + bool isOnRunLoopThread() const noexcept override + { + return true; + } + + private: + void startObserving() const noexcept override {} + void stopObserving() const noexcept override {} +}; + +class BridgelessFeatureFlags : public ReactNativeFeatureFlagsDefaults { + public: + bool enableBridgelessArchitecture() override + { + return true; + } +}; + +/* + * A real JavaScript-thread stand-in: `induce` blocks the calling thread until + * another thread hands it the runtime, so the executor cannot run inline. + */ +class RuntimeThread { + public: + RuntimeThread() + : runtime_(facebook::hermes::makeHermesRuntime(::hermes::vm::RuntimeConfig::Builder().build())), + thread_([this] { loop(); }) + { + } + + ~RuntimeThread() + { + { + std::lock_guard lock(mutex_); + stopped_ = true; + } + condition_.notify_all(); + thread_.join(); + } + + RuntimeExecutor executor() + { + return [this](std::function &&callback) { + { + std::lock_guard lock(mutex_); + queue_.push_back(std::move(callback)); + } + condition_.notify_all(); + }; + } + + private: + void loop() + { + std::unique_lock lock(mutex_); + while (true) { + condition_.wait(lock, [this] { return stopped_ || !queue_.empty(); }); + if (queue_.empty()) { + return; + } + auto callback = std::move(queue_.front()); + queue_.pop_front(); + lock.unlock(); + callback(*runtime_); + lock.lock(); + } + } + + std::unique_ptr runtime_; + std::deque> queue_; + std::mutex mutex_; + std::condition_variable condition_; + bool stopped_{false}; + std::thread thread_; +}; + +} // namespace + +@interface AppleEventBeatTests : XCTestCase +@end + +@implementation AppleEventBeatTests { + std::unique_ptr _runtimeThread; + std::unique_ptr _runtimeScheduler; + std::shared_ptr _ownerBox; + std::shared_ptr _owner; + std::shared_ptr _beatCount; + std::unique_ptr _eventBeat; + CALayer *_hostLayer; +} + +- (void)setUp +{ + [super setUp]; + + ReactNativeFeatureFlags::dangerouslyReset(); + ReactNativeFeatureFlags::override(std::make_unique()); + + _runtimeThread = std::make_unique(); + _runtimeScheduler = std::make_unique(_runtimeThread->executor()); + + _owner = std::make_shared(0); + _ownerBox = std::make_shared(); + _ownerBox->owner = _owner; + + _hostLayer = [CALayer layer]; + CALayer *hostLayer = _hostLayer; + + auto runLoopObserver = + std::make_unique(RunLoopObserver::Activity::BeforeWaiting, _ownerBox->owner); + _eventBeat = std::make_unique( + _ownerBox, std::move(runLoopObserver), *_runtimeScheduler, [hostLayer](Tag) -> CALayer * { return hostLayer; }); + + _beatCount = std::make_shared(0); + auto beatCount = _beatCount; + _eventBeat->setBeatCallback([beatCount](facebook::jsi::Runtime &) { (*beatCount)++; }); +} + +- (void)tearDown +{ + _eventBeat.reset(); + _runtimeScheduler.reset(); + _runtimeThread.reset(); + ReactNativeFeatureFlags::dangerouslyReset(); + [super tearDown]; +} + +- (CALayer *)flusherLayer +{ + XCTAssertEqual(_hostLayer.sublayers.count, 1u, @"the request should have attached a flusher to the window layer"); + return _hostLayer.sublayers.firstObject; +} + +- (void)testRequestAttachesAFlusherToTheResolvedWindowLayer +{ + XCTAssertEqual(_hostLayer.sublayers.count, 0u); + _eventBeat->requestSynchronous(42); + XCTAssertEqual(_hostLayer.sublayers.count, 1u); + + // A second request reuses the same flusher rather than stacking layers. + _eventBeat->requestSynchronous(43); + XCTAssertEqual(_hostLayer.sublayers.count, 1u); +} + +- (void)testDisplayInducesTheBeat +{ + _eventBeat->requestSynchronous(42); + [self.flusherLayer display]; + XCTAssertEqual(*_beatCount, 1); +} + +- (void)testAtMostOneDisplayInducePerRunLoopTurn +{ + _eventBeat->requestSynchronous(42); + CALayer *flusher = self.flusherLayer; + + [flusher display]; + XCTAssertEqual(*_beatCount, 1); + + // What the beat does during the first induce — mount, which re-runs layout, + // which lets a layout-driven emitter request again — re-enters the display + // phase of the same commit. Core Animation puts no bound on that, so the + // beat must: the second display does not induce. + _eventBeat->requestSynchronous(42); + [flusher display]; + XCTAssertEqual(*_beatCount, 1, @"a second display in the same run loop turn must not induce again"); +} + +- (void)testInducesAgainInTheNextRunLoopTurn +{ + _eventBeat->requestSynchronous(42); + CALayer *flusher = self.flusherLayer; + [flusher display]; + XCTAssertEqual(*_beatCount, 1); + + // The run loop observer runs before Core Animation's commit observer, so it + // opens the next turn. The previous request was already consumed, so the + // observer's own induce is a no-op. + _eventBeat->activityDidChange(_eventBeat.get(), RunLoopObserver::Activity::BeforeWaiting); + XCTAssertEqual(*_beatCount, 1); + + _eventBeat->requestSynchronous(42); + [flusher display]; + XCTAssertEqual(*_beatCount, 2, @"the bound is per turn, not for the lifetime of the beat"); +} + +- (void)testRequestWithoutATagDoesNotAttachAFlusher +{ + _eventBeat->requestSynchronous(kNoTag); + XCTAssertEqual(_hostLayer.sublayers.count, 0u); +} + +@end