Skip to content

Commit 18f4db4

Browse files
rubennortefacebook-github-bot
authored andcommitted
Clean up dead logic in RuntimeScheduler_Modern and simplify logic (#52192)
Summary: Pull Request resolved: #52192 Changelog: [internal] When we shipped the event loop we removed the need to run only expired tasks in `RuntimeScheduler_Modern`, but we never cleaned up the code properly. This does it. Reviewed By: javache Differential Revision: D77142978 fbshipit-source-id: f808edf80a134f487723fa36ab7a3593e4efe2d3
1 parent 251eb3f commit 18f4db4

File tree

2 files changed

+16
-34
lines changed

2 files changed

+16
-34
lines changed

packages/react-native/ReactCommon/react/renderer/runtimescheduler/RuntimeScheduler_Modern.cpp

Lines changed: 13 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -111,10 +111,10 @@ std::shared_ptr<Task> RuntimeScheduler_Modern::scheduleIdleTask(
111111
}
112112

113113
bool RuntimeScheduler_Modern::getShouldYield() noexcept {
114-
std::shared_lock lock(schedulingMutex_);
115-
116114
markYieldingOpportunity(now_());
117115

116+
std::shared_lock lock(schedulingMutex_);
117+
118118
return syncTaskRequests_ > 0 ||
119119
(!taskQueue_.empty() && taskQueue_.top().get() != currentTask_);
120120
}
@@ -152,13 +152,12 @@ void RuntimeScheduler_Modern::executeNowOnTheSameThread(
152152

153153
syncTaskRequests_++;
154154
executeSynchronouslyOnSameThread_CAN_DEADLOCK(
155-
runtimeExecutor_,
156-
[this, currentTime, &task](jsi::Runtime& runtime) mutable {
155+
runtimeExecutor_, [&](jsi::Runtime& runtime) mutable {
157156
TraceSection s2("RuntimeScheduler::executeNowOnTheSameThread callback");
158157

159158
syncTaskRequests_--;
160159
runtimePtr = &runtime;
161-
runEventLoopTick(runtime, task, currentTime);
160+
runEventLoopTick(runtime, task);
162161
runtimePtr = nullptr;
163162
});
164163

@@ -249,37 +248,29 @@ void RuntimeScheduler_Modern::scheduleTask(std::shared_ptr<Task> task) {
249248
}
250249

251250
void RuntimeScheduler_Modern::scheduleEventLoop() {
252-
runtimeExecutor_(
253-
[this](jsi::Runtime& runtime) { runEventLoop(runtime, false); });
251+
runtimeExecutor_([this](jsi::Runtime& runtime) { runEventLoop(runtime); });
254252
}
255253

256-
void RuntimeScheduler_Modern::runEventLoop(
257-
jsi::Runtime& runtime,
258-
bool onlyExpired) {
254+
void RuntimeScheduler_Modern::runEventLoop(jsi::Runtime& runtime) {
259255
TraceSection s("RuntimeScheduler::runEventLoop");
260256

261257
auto previousPriority = currentPriority_;
262258

263-
auto currentTime = now_();
264259
// `selectTask` must be called unconditionaly to ensure that
265260
// `isEventLoopScheduled_` is set to false and the event loop resume
266261
// correctly if a synchronous task is scheduled.
267262
// Unit test normalTaskYieldsToSynchronousAccessAndResumes covers this
268263
// scenario.
269-
auto topPriorityTask = selectTask(currentTime, onlyExpired);
264+
auto topPriorityTask = selectTask();
270265
while (topPriorityTask && syncTaskRequests_ == 0) {
271-
runEventLoopTick(runtime, *topPriorityTask, currentTime);
272-
273-
currentTime = now_();
274-
topPriorityTask = selectTask(currentTime, onlyExpired);
266+
runEventLoopTick(runtime, *topPriorityTask);
267+
topPriorityTask = selectTask();
275268
}
276269

277270
currentPriority_ = previousPriority;
278271
}
279272

280-
std::shared_ptr<Task> RuntimeScheduler_Modern::selectTask(
281-
HighResTimeStamp currentTime,
282-
bool onlyExpired) {
273+
std::shared_ptr<Task> RuntimeScheduler_Modern::selectTask() {
283274
// We need a unique lock here because we'll also remove executed tasks from
284275
// the top of the queue.
285276
std::unique_lock lock(schedulingMutex_);
@@ -294,20 +285,15 @@ std::shared_ptr<Task> RuntimeScheduler_Modern::selectTask(
294285
}
295286

296287
if (!taskQueue_.empty()) {
297-
auto task = taskQueue_.top();
298-
auto didUserCallbackTimeout = task->expirationTime <= currentTime;
299-
if (!onlyExpired || didUserCallbackTimeout) {
300-
return task;
301-
}
288+
return taskQueue_.top();
302289
}
303290

304291
return nullptr;
305292
}
306293

307294
void RuntimeScheduler_Modern::runEventLoopTick(
308295
jsi::Runtime& runtime,
309-
Task& task,
310-
HighResTimeStamp taskStartTime) {
296+
Task& task) {
311297
TraceSection s("RuntimeScheduler::runEventLoopTick");
312298
jsinspector_modern::tracing::EventLoopReporter performanceReporter(
313299
jsinspector_modern::tracing::EventLoopPhase::Task);
@@ -318,6 +304,7 @@ void RuntimeScheduler_Modern::runEventLoopTick(
318304
currentTask_ = &task;
319305
currentPriority_ = task.priority;
320306

307+
auto taskStartTime = now_();
321308
lastYieldingOpportunity_ = taskStartTime;
322309
longestPeriodWithoutYieldingOpportunity_ = HighResDuration::zero();
323310

packages/react-native/ReactCommon/react/renderer/runtimescheduler/RuntimeScheduler_Modern.h

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -179,11 +179,9 @@ class RuntimeScheduler_Modern final : public RuntimeSchedulerBase {
179179
SchedulerPriority currentPriority_{SchedulerPriority::NormalPriority};
180180

181181
void scheduleEventLoop();
182-
void runEventLoop(jsi::Runtime& runtime, bool onlyExpired);
182+
void runEventLoop(jsi::Runtime& runtime);
183183

184-
std::shared_ptr<Task> selectTask(
185-
HighResTimeStamp currentTime,
186-
bool onlyExpired);
184+
std::shared_ptr<Task> selectTask();
187185

188186
void scheduleTask(std::shared_ptr<Task> task);
189187

@@ -193,10 +191,7 @@ class RuntimeScheduler_Modern final : public RuntimeSchedulerBase {
193191
* In the future, this will include other steps in the Web event loop, like
194192
* updating the UI in native, executing resize observer callbacks, etc.
195193
*/
196-
void runEventLoopTick(
197-
jsi::Runtime& runtime,
198-
Task& task,
199-
HighResTimeStamp taskStartTime);
194+
void runEventLoopTick(jsi::Runtime& runtime, Task& task);
200195

201196
void executeTask(
202197
jsi::Runtime& runtime,

0 commit comments

Comments
 (0)