Skip to content

Commit 11f4df0

Browse files
Abbondanzofacebook-github-bot
authored andcommitted
Bound the display-phase event beat induce to one per run loop turn
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 Differential Revision: D120951592
1 parent a016535 commit 11f4df0

3 files changed

Lines changed: 262 additions & 0 deletions

File tree

‎packages/react-native/React/Fabric/AppleEventBeat.h‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,20 @@ class AppleEventBeat : public EventBeat, public RunLoopObserver::Delegate {
7070
WindowLayerResolver windowLayerResolver_;
7171
NSMapTable<CALayer *, RCTEventBeatFlusherLayer *> *layers_;
7272
void (^onDisplay_)(void);
73+
74+
/*
75+
* Whether a display phase has already induced in the current run loop turn.
76+
*
77+
* The beat a display phase induces runs the whole event loop tick on the
78+
* main thread, mounting included, so a `VirtualView` whose layout metrics
79+
* change during that mount emits another synchronous request from inside the
80+
* display it is servicing. Core Animation honours `setNeedsDisplay` made
81+
* during a display by running the commit's layout and display phases again,
82+
* with no bound, so without this flag one commit can perform an unbounded
83+
* number of blocking JavaScript round trips. Requests that arrive after the
84+
* first induce keep the ordinary run loop observer timing.
85+
*/
86+
mutable bool didInduceInCurrentTurn_{false};
7387
};
7488

7589
} // namespace facebook::react

‎packages/react-native/React/Fabric/AppleEventBeat.mm‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,10 @@ - (void)display
6969
if (!owner) {
7070
return;
7171
}
72+
if (this->didInduceInCurrentTurn_) {
73+
return;
74+
}
75+
this->didInduceInCurrentTurn_ = true;
7276
this->induce();
7377
};
7478

@@ -117,6 +121,9 @@ - (void)display
117121
RunLoopObserver::Activity /*activity*/) const noexcept
118122
{
119123
react_native_assert(delegate == this);
124+
// This observer runs before Core Animation's commit observer, so it is the
125+
// start of the turn whose display phase `didInduceInCurrentTurn_` bounds.
126+
didInduceInCurrentTurn_ = false;
120127
induce();
121128
}
122129

Lines changed: 241 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,241 @@
1+
/*
2+
* Copyright (c) Meta Platforms, Inc. and affiliates.
3+
*
4+
* This source code is licensed under the MIT license found in the
5+
* LICENSE file in the root directory of this source tree.
6+
*/
7+
8+
#import <QuartzCore/QuartzCore.h>
9+
#import <XCTest/XCTest.h>
10+
11+
#import <react/featureflags/ReactNativeFeatureFlags.h>
12+
#import <react/featureflags/ReactNativeFeatureFlagsDefaults.h>
13+
#import <react/renderer/runtimescheduler/RuntimeScheduler.h>
14+
15+
#import <hermes/hermes.h>
16+
17+
#import <condition_variable>
18+
#import <deque>
19+
#import <memory>
20+
#import <mutex>
21+
#import <thread>
22+
23+
// `AppleEventBeat.h` is private to `RCTFabric` and is not reachable from a
24+
// dependent target, so the test target declares it as its own header rather
25+
// than the library exporting it for one test's sake.
26+
#import "AppleEventBeat.h"
27+
28+
using facebook::react::AppleEventBeat;
29+
using facebook::react::EventBeat;
30+
using facebook::react::kNoTag;
31+
using facebook::react::ReactNativeFeatureFlags;
32+
using facebook::react::ReactNativeFeatureFlagsDefaults;
33+
using facebook::react::RunLoopObserver;
34+
using facebook::react::RuntimeExecutor;
35+
using facebook::react::RuntimeScheduler;
36+
using facebook::react::Tag;
37+
38+
namespace {
39+
40+
/*
41+
* The beat only needs an observer it can own; the tests drive the turn
42+
* boundary through `activityDidChange` directly, which is what the real
43+
* observer does.
44+
*/
45+
class StubRunLoopObserver : public RunLoopObserver {
46+
public:
47+
using RunLoopObserver::RunLoopObserver;
48+
49+
bool isOnRunLoopThread() const noexcept override
50+
{
51+
return true;
52+
}
53+
54+
private:
55+
void startObserving() const noexcept override {}
56+
void stopObserving() const noexcept override {}
57+
};
58+
59+
class BridgelessFeatureFlags : public ReactNativeFeatureFlagsDefaults {
60+
public:
61+
bool enableBridgelessArchitecture() override
62+
{
63+
return true;
64+
}
65+
};
66+
67+
/*
68+
* A real JavaScript-thread stand-in: `induce` blocks the calling thread until
69+
* another thread hands it the runtime, so the executor cannot run inline.
70+
*/
71+
class RuntimeThread {
72+
public:
73+
RuntimeThread()
74+
: runtime_(facebook::hermes::makeHermesRuntime(::hermes::vm::RuntimeConfig::Builder().build())),
75+
thread_([this] { loop(); })
76+
{
77+
}
78+
79+
~RuntimeThread()
80+
{
81+
{
82+
std::lock_guard<std::mutex> lock(mutex_);
83+
stopped_ = true;
84+
}
85+
condition_.notify_all();
86+
thread_.join();
87+
}
88+
89+
RuntimeExecutor executor()
90+
{
91+
return [this](std::function<void(facebook::jsi::Runtime &)> &&callback) {
92+
{
93+
std::lock_guard<std::mutex> lock(mutex_);
94+
queue_.push_back(std::move(callback));
95+
}
96+
condition_.notify_all();
97+
};
98+
}
99+
100+
private:
101+
void loop()
102+
{
103+
std::unique_lock<std::mutex> lock(mutex_);
104+
while (true) {
105+
condition_.wait(lock, [this] { return stopped_ || !queue_.empty(); });
106+
if (queue_.empty()) {
107+
return;
108+
}
109+
auto callback = std::move(queue_.front());
110+
queue_.pop_front();
111+
lock.unlock();
112+
callback(*runtime_);
113+
lock.lock();
114+
}
115+
}
116+
117+
std::unique_ptr<facebook::hermes::HermesRuntime> runtime_;
118+
std::deque<std::function<void(facebook::jsi::Runtime &)>> queue_;
119+
std::mutex mutex_;
120+
std::condition_variable condition_;
121+
bool stopped_{false};
122+
std::thread thread_;
123+
};
124+
125+
} // namespace
126+
127+
@interface AppleEventBeatTests : XCTestCase
128+
@end
129+
130+
@implementation AppleEventBeatTests {
131+
std::unique_ptr<RuntimeThread> _runtimeThread;
132+
std::unique_ptr<RuntimeScheduler> _runtimeScheduler;
133+
std::shared_ptr<EventBeat::OwnerBox> _ownerBox;
134+
std::shared_ptr<int> _owner;
135+
std::shared_ptr<int> _beatCount;
136+
std::unique_ptr<AppleEventBeat> _eventBeat;
137+
CALayer *_hostLayer;
138+
}
139+
140+
- (void)setUp
141+
{
142+
[super setUp];
143+
144+
ReactNativeFeatureFlags::dangerouslyReset();
145+
ReactNativeFeatureFlags::override(std::make_unique<BridgelessFeatureFlags>());
146+
147+
_runtimeThread = std::make_unique<RuntimeThread>();
148+
_runtimeScheduler = std::make_unique<RuntimeScheduler>(_runtimeThread->executor());
149+
150+
_owner = std::make_shared<int>(0);
151+
_ownerBox = std::make_shared<EventBeat::OwnerBox>();
152+
_ownerBox->owner = _owner;
153+
154+
_hostLayer = [CALayer layer];
155+
CALayer *hostLayer = _hostLayer;
156+
157+
auto runLoopObserver =
158+
std::make_unique<const StubRunLoopObserver>(RunLoopObserver::Activity::BeforeWaiting, _ownerBox->owner);
159+
_eventBeat = std::make_unique<AppleEventBeat>(
160+
_ownerBox, std::move(runLoopObserver), *_runtimeScheduler, [hostLayer](Tag) -> CALayer * { return hostLayer; });
161+
162+
_beatCount = std::make_shared<int>(0);
163+
auto beatCount = _beatCount;
164+
_eventBeat->setBeatCallback([beatCount](facebook::jsi::Runtime &) { (*beatCount)++; });
165+
}
166+
167+
- (void)tearDown
168+
{
169+
_eventBeat.reset();
170+
_runtimeScheduler.reset();
171+
_runtimeThread.reset();
172+
ReactNativeFeatureFlags::dangerouslyReset();
173+
[super tearDown];
174+
}
175+
176+
- (CALayer *)flusherLayer
177+
{
178+
XCTAssertEqual(_hostLayer.sublayers.count, 1u, @"the request should have attached a flusher to the window layer");
179+
return _hostLayer.sublayers.firstObject;
180+
}
181+
182+
- (void)testRequestAttachesAFlusherToTheResolvedWindowLayer
183+
{
184+
XCTAssertEqual(_hostLayer.sublayers.count, 0u);
185+
_eventBeat->requestSynchronous(42);
186+
XCTAssertEqual(_hostLayer.sublayers.count, 1u);
187+
188+
// A second request reuses the same flusher rather than stacking layers.
189+
_eventBeat->requestSynchronous(43);
190+
XCTAssertEqual(_hostLayer.sublayers.count, 1u);
191+
}
192+
193+
- (void)testDisplayInducesTheBeat
194+
{
195+
_eventBeat->requestSynchronous(42);
196+
[self.flusherLayer display];
197+
XCTAssertEqual(*_beatCount, 1);
198+
}
199+
200+
- (void)testAtMostOneDisplayInducePerRunLoopTurn
201+
{
202+
_eventBeat->requestSynchronous(42);
203+
CALayer *flusher = self.flusherLayer;
204+
205+
[flusher display];
206+
XCTAssertEqual(*_beatCount, 1);
207+
208+
// What the beat does during the first induce — mount, which re-runs layout,
209+
// which lets a layout-driven emitter request again — re-enters the display
210+
// phase of the same commit. Core Animation puts no bound on that, so the
211+
// beat must: the second display does not induce.
212+
_eventBeat->requestSynchronous(42);
213+
[flusher display];
214+
XCTAssertEqual(*_beatCount, 1, @"a second display in the same run loop turn must not induce again");
215+
}
216+
217+
- (void)testInducesAgainInTheNextRunLoopTurn
218+
{
219+
_eventBeat->requestSynchronous(42);
220+
CALayer *flusher = self.flusherLayer;
221+
[flusher display];
222+
XCTAssertEqual(*_beatCount, 1);
223+
224+
// The run loop observer runs before Core Animation's commit observer, so it
225+
// opens the next turn. The previous request was already consumed, so the
226+
// observer's own induce is a no-op.
227+
_eventBeat->activityDidChange(_eventBeat.get(), RunLoopObserver::Activity::BeforeWaiting);
228+
XCTAssertEqual(*_beatCount, 1);
229+
230+
_eventBeat->requestSynchronous(42);
231+
[flusher display];
232+
XCTAssertEqual(*_beatCount, 2, @"the bound is per turn, not for the lifetime of the beat");
233+
}
234+
235+
- (void)testRequestWithoutATagDoesNotAttachAFlusher
236+
{
237+
_eventBeat->requestSynchronous(kNoTag);
238+
XCTAssertEqual(_hostLayer.sublayers.count, 0u);
239+
}
240+
241+
@end

0 commit comments

Comments
 (0)