Skip to content

Commit

Permalink
Small refactor in RuntimeScheduler_Modern to favor references over sh…
Browse files Browse the repository at this point in the history
…ared_ptr for non-owning function args (facebook#43852)

Summary:
Pull Request resolved: facebook#43852

Changelog: [internal]

Just a small refactor so we rely less on shared pointers within `RuntimeSCheduler_Modern`.

Reviewed By: javache

Differential Revision: D55646389
  • Loading branch information
rubennorte authored and facebook-github-bot committed Apr 9, 2024
1 parent 90d3dac commit 65d888e
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 15 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ bool RuntimeScheduler_Modern::getShouldYield() const noexcept {
std::shared_lock lock(schedulingMutex_);

return syncTaskRequests_ > 0 ||
(!taskQueue_.empty() && taskQueue_.top() != currentTask_);
(!taskQueue_.empty() && taskQueue_.top().get() != currentTask_);
}

void RuntimeScheduler_Modern::cancelTask(Task& task) noexcept {
Expand Down Expand Up @@ -105,9 +105,8 @@ void RuntimeScheduler_Modern::executeNowOnTheSameThread(
auto priority = SchedulerPriority::ImmediatePriority;
auto expirationTime =
currentTime + timeoutForSchedulerPriority(priority);
auto task = std::make_shared<Task>(
priority, std::move(callback), expirationTime);

auto task = Task{priority, std::move(callback), expirationTime};
executeTask(runtime, task, currentTime);
});

Expand Down Expand Up @@ -207,7 +206,7 @@ void RuntimeScheduler_Modern::startWorkLoop(
break;
}

executeTask(runtime, topPriorityTask, currentTime);
executeTask(runtime, *topPriorityTask, currentTime);
}
} catch (jsi::JSError& error) {
handleJSError(runtime, error, true);
Expand Down Expand Up @@ -245,19 +244,19 @@ std::shared_ptr<Task> RuntimeScheduler_Modern::selectTask(

void RuntimeScheduler_Modern::executeTask(
jsi::Runtime& runtime,
const std::shared_ptr<Task>& task,
Task& task,
RuntimeSchedulerTimePoint currentTime) {
auto didUserCallbackTimeout = task->expirationTime <= currentTime;
auto didUserCallbackTimeout = task.expirationTime <= currentTime;

SystraceSection s(
"RuntimeScheduler::executeTask",
"priority",
serialize(task->priority),
serialize(task.priority),
"didUserCallbackTimeout",
didUserCallbackTimeout);

currentTask_ = task;
currentPriority_ = task->priority;
currentTask_ = &task;
currentPriority_ = task.priority;

{
ScopedShadowTreeRevisionLock revisionLock(
Expand All @@ -275,6 +274,8 @@ void RuntimeScheduler_Modern::executeTask(
updateRendering();
}
}

currentTask_ = nullptr;
}

/**
Expand All @@ -296,16 +297,16 @@ void RuntimeScheduler_Modern::updateRendering() {

void RuntimeScheduler_Modern::executeMacrotask(
jsi::Runtime& runtime,
std::shared_ptr<Task> task,
Task& task,
bool didUserCallbackTimeout) const {
SystraceSection s("RuntimeScheduler::executeMacrotask");

auto result = task->execute(runtime, didUserCallbackTimeout);
auto result = task.execute(runtime, didUserCallbackTimeout);

if (result.isObject() && result.getObject(runtime).isFunction(runtime)) {
// If the task returned a continuation callback, we re-assign it to the task
// and keep the task in the queue.
task->callback = result.getObject(runtime).getFunction(runtime);
task.callback = result.getObject(runtime).getFunction(runtime);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ class RuntimeScheduler_Modern final : public RuntimeSchedulerBase {
TaskPriorityComparer>
taskQueue_;

std::shared_ptr<Task> currentTask_;
Task* currentTask_{};

/**
* This protects the access to `taskQueue_` and `isWorkLoopScheduled_`.
Expand All @@ -162,12 +162,12 @@ class RuntimeScheduler_Modern final : public RuntimeSchedulerBase {
*/
void executeTask(
jsi::Runtime& runtime,
const std::shared_ptr<Task>& task,
Task& task,
RuntimeSchedulerTimePoint currentTime);

void executeMacrotask(
jsi::Runtime& runtime,
std::shared_ptr<Task> task,
Task& task,
bool didUserCallbackTimeout) const;

void updateRendering();
Expand Down

0 comments on commit 65d888e

Please sign in to comment.