Skip to content

Commit

Permalink
Clean up const and mutable modifiers in RuntimeScheduler (#41028)
Browse files Browse the repository at this point in the history
Summary:
Pull Request resolved: #41028

This removes misleading `const` modifiers from some methods in `RuntimeScheduler` that shouldn't really use it, and removes the `mutable` modifiers that were only necessary because of that.

Changelog: [internal]

Reviewed By: sammy-SC

Differential Revision: D50364626

fbshipit-source-id: 28ed9fa923f8e787166f702ccaecd41a635d3b3a
  • Loading branch information
rubennorte authored and facebook-github-bot committed Oct 18, 2023
1 parent 87c08df commit 87dbe44
Show file tree
Hide file tree
Showing 6 changed files with 49 additions and 57 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -40,19 +40,19 @@ RuntimeScheduler::RuntimeScheduler(
useModernRuntimeScheduler,
std::move(now))) {}

void RuntimeScheduler::scheduleWork(RawCallback&& callback) const noexcept {
void RuntimeScheduler::scheduleWork(RawCallback&& callback) noexcept {
return runtimeSchedulerImpl_->scheduleWork(std::move(callback));
}

std::shared_ptr<Task> RuntimeScheduler::scheduleTask(
SchedulerPriority priority,
jsi::Function&& callback) const noexcept {
jsi::Function&& callback) noexcept {
return runtimeSchedulerImpl_->scheduleTask(priority, std::move(callback));
}

std::shared_ptr<Task> RuntimeScheduler::scheduleTask(
SchedulerPriority priority,
RawCallback&& callback) const noexcept {
RawCallback&& callback) noexcept {
return runtimeSchedulerImpl_->scheduleTask(priority, std::move(callback));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,20 +18,14 @@ namespace facebook::react {
class RuntimeSchedulerBase {
public:
virtual ~RuntimeSchedulerBase() = default;
// FIXME(T167271466): remove `const` modified when the RuntimeScheduler
// refactor has been shipped.
virtual void scheduleWork(RawCallback&& callback) const noexcept = 0;
virtual void scheduleWork(RawCallback&& callback) noexcept = 0;
virtual void executeNowOnTheSameThread(RawCallback&& callback) = 0;
// FIXME(T167271466): remove `const` modified when the RuntimeScheduler
// refactor has been shipped.
virtual std::shared_ptr<Task> scheduleTask(
SchedulerPriority priority,
jsi::Function&& callback) const noexcept = 0;
// FIXME(T167271466): remove `const` modified when the RuntimeScheduler
// refactor has been shipped.
jsi::Function&& callback) noexcept = 0;
virtual std::shared_ptr<Task> scheduleTask(
SchedulerPriority priority,
RawCallback&& callback) const noexcept = 0;
RawCallback&& callback) noexcept = 0;
virtual void cancelTask(Task& task) noexcept = 0;
virtual bool getShouldYield() const noexcept = 0;
virtual bool getIsSynchronous() const noexcept = 0;
Expand Down Expand Up @@ -62,7 +56,7 @@ class RuntimeScheduler final : RuntimeSchedulerBase {
RuntimeScheduler(RuntimeScheduler&&) = delete;
RuntimeScheduler& operator=(RuntimeScheduler&&) = delete;

void scheduleWork(RawCallback&& callback) const noexcept override;
void scheduleWork(RawCallback&& callback) noexcept override;

/*
* Grants access to the runtime synchronously on the caller's thread.
Expand All @@ -81,11 +75,11 @@ class RuntimeScheduler final : RuntimeSchedulerBase {
*/
std::shared_ptr<Task> scheduleTask(
SchedulerPriority priority,
jsi::Function&& callback) const noexcept override;
jsi::Function&& callback) noexcept override;

std::shared_ptr<Task> scheduleTask(
SchedulerPriority priority,
RawCallback&& callback) const noexcept override;
RawCallback&& callback) noexcept override;

/*
* Cancelled task will never be executed.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,7 @@ RuntimeScheduler_Legacy::RuntimeScheduler_Legacy(
std::function<RuntimeSchedulerTimePoint()> now)
: runtimeExecutor_(std::move(runtimeExecutor)), now_(std::move(now)) {}

void RuntimeScheduler_Legacy::scheduleWork(
RawCallback&& callback) const noexcept {
void RuntimeScheduler_Legacy::scheduleWork(RawCallback&& callback) noexcept {
SystraceSection s("RuntimeScheduler::scheduleWork");

runtimeAccessRequests_ += 1;
Expand All @@ -38,7 +37,7 @@ void RuntimeScheduler_Legacy::scheduleWork(

std::shared_ptr<Task> RuntimeScheduler_Legacy::scheduleTask(
SchedulerPriority priority,
jsi::Function&& callback) const noexcept {
jsi::Function&& callback) noexcept {
SystraceSection s(
"RuntimeScheduler::scheduleTask",
"priority",
Expand All @@ -58,7 +57,7 @@ std::shared_ptr<Task> RuntimeScheduler_Legacy::scheduleTask(

std::shared_ptr<Task> RuntimeScheduler_Legacy::scheduleTask(
SchedulerPriority priority,
RawCallback&& callback) const noexcept {
RawCallback&& callback) noexcept {
SystraceSection s(
"RuntimeScheduler::scheduleTask",
"priority",
Expand Down Expand Up @@ -145,7 +144,7 @@ void RuntimeScheduler_Legacy::callExpiredTasks(jsi::Runtime& runtime) {

#pragma mark - Private

void RuntimeScheduler_Legacy::scheduleWorkLoopIfNecessary() const {
void RuntimeScheduler_Legacy::scheduleWorkLoopIfNecessary() {
if (!isWorkLoopScheduled_ && !isPerformingWork_) {
isWorkLoopScheduled_ = true;
runtimeExecutor_([this](jsi::Runtime& runtime) {
Expand All @@ -155,7 +154,7 @@ void RuntimeScheduler_Legacy::scheduleWorkLoopIfNecessary() const {
}
}

void RuntimeScheduler_Legacy::startWorkLoop(jsi::Runtime& runtime) const {
void RuntimeScheduler_Legacy::startWorkLoop(jsi::Runtime& runtime) {
SystraceSection s("RuntimeScheduler::startWorkLoop");

auto previousPriority = currentPriority_;
Expand Down Expand Up @@ -184,7 +183,7 @@ void RuntimeScheduler_Legacy::startWorkLoop(jsi::Runtime& runtime) const {
void RuntimeScheduler_Legacy::executeTask(
jsi::Runtime& runtime,
const std::shared_ptr<Task>& task,
bool didUserCallbackTimeout) const {
bool didUserCallbackTimeout) {
SystraceSection s(
"RuntimeScheduler::executeTask",
"priority",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ class RuntimeScheduler_Legacy final : public RuntimeSchedulerBase {
RuntimeScheduler_Legacy(RuntimeScheduler_Legacy&&) = delete;
RuntimeScheduler_Legacy& operator=(RuntimeScheduler_Legacy&&) = delete;

void scheduleWork(RawCallback&& callback) const noexcept override;
void scheduleWork(RawCallback&& callback) noexcept override;

/*
* Grants access to the runtime synchronously on the caller's thread.
Expand All @@ -55,11 +55,11 @@ class RuntimeScheduler_Legacy final : public RuntimeSchedulerBase {
*/
std::shared_ptr<Task> scheduleTask(
SchedulerPriority priority,
jsi::Function&& callback) const noexcept override;
jsi::Function&& callback) noexcept override;

std::shared_ptr<Task> scheduleTask(
SchedulerPriority priority,
RawCallback&& callback) const noexcept override;
RawCallback&& callback) noexcept override;

/*
* Cancelled task will never be executed.
Expand Down Expand Up @@ -111,34 +111,34 @@ class RuntimeScheduler_Legacy final : public RuntimeSchedulerBase {
void callExpiredTasks(jsi::Runtime& runtime) override;

private:
mutable std::priority_queue<
std::priority_queue<
std::shared_ptr<Task>,
std::vector<std::shared_ptr<Task>>,
TaskPriorityComparer>
taskQueue_;

const RuntimeExecutor runtimeExecutor_;
mutable SchedulerPriority currentPriority_{SchedulerPriority::NormalPriority};
SchedulerPriority currentPriority_{SchedulerPriority::NormalPriority};

/*
* Counter indicating how many access to the runtime have been requested.
*/
mutable std::atomic<uint_fast8_t> runtimeAccessRequests_{0};
std::atomic<uint_fast8_t> runtimeAccessRequests_{0};

mutable std::atomic_bool isSynchronous_{false};
std::atomic_bool isSynchronous_{false};

void startWorkLoop(jsi::Runtime& runtime) const;
void startWorkLoop(jsi::Runtime& runtime);

/*
* Schedules a work loop unless it has been already scheduled
* This is to avoid unnecessary calls to `runtimeExecutor`.
*/
void scheduleWorkLoopIfNecessary() const;
void scheduleWorkLoopIfNecessary();

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

/*
* Returns a time point representing the current point in time. May be called
Expand All @@ -150,12 +150,12 @@ class RuntimeScheduler_Legacy final : public RuntimeSchedulerBase {
* Flag indicating if callback on JavaScript queue has been
* scheduled.
*/
mutable std::atomic_bool isWorkLoopScheduled_{false};
std::atomic_bool isWorkLoopScheduled_{false};

/*
* This flag is set while performing work, to prevent re-entrancy.
*/
mutable std::atomic_bool isPerformingWork_{false};
std::atomic_bool isPerformingWork_{false};
};

} // namespace facebook::react
Original file line number Diff line number Diff line change
Expand Up @@ -21,15 +21,14 @@ RuntimeScheduler_Modern::RuntimeScheduler_Modern(
std::function<RuntimeSchedulerTimePoint()> now)
: runtimeExecutor_(std::move(runtimeExecutor)), now_(std::move(now)) {}

void RuntimeScheduler_Modern::scheduleWork(
RawCallback&& callback) const noexcept {
void RuntimeScheduler_Modern::scheduleWork(RawCallback&& callback) noexcept {
SystraceSection s("RuntimeScheduler::scheduleWork");
scheduleTask(SchedulerPriority::ImmediatePriority, std::move(callback));
}

std::shared_ptr<Task> RuntimeScheduler_Modern::scheduleTask(
SchedulerPriority priority,
jsi::Function&& callback) const noexcept {
jsi::Function&& callback) noexcept {
SystraceSection s(
"RuntimeScheduler::scheduleTask",
"priority",
Expand All @@ -48,7 +47,7 @@ std::shared_ptr<Task> RuntimeScheduler_Modern::scheduleTask(

std::shared_ptr<Task> RuntimeScheduler_Modern::scheduleTask(
SchedulerPriority priority,
RawCallback&& callback) const noexcept {
RawCallback&& callback) noexcept {
SystraceSection s(
"RuntimeScheduler::scheduleTask",
"priority",
Expand Down Expand Up @@ -144,7 +143,7 @@ void RuntimeScheduler_Modern::callExpiredTasks(jsi::Runtime& runtime) {

#pragma mark - Private

void RuntimeScheduler_Modern::scheduleTask(std::shared_ptr<Task> task) const {
void RuntimeScheduler_Modern::scheduleTask(std::shared_ptr<Task> task) {
bool shouldScheduleWorkLoop = false;

{
Expand All @@ -167,14 +166,14 @@ void RuntimeScheduler_Modern::scheduleTask(std::shared_ptr<Task> task) const {
}
}

void RuntimeScheduler_Modern::scheduleWorkLoop() const {
void RuntimeScheduler_Modern::scheduleWorkLoop() {
runtimeExecutor_(
[this](jsi::Runtime& runtime) { startWorkLoop(runtime, false); });
}

void RuntimeScheduler_Modern::startWorkLoop(
jsi::Runtime& runtime,
bool onlyExpired) const {
bool onlyExpired) {
SystraceSection s("RuntimeScheduler::startWorkLoop");

auto previousPriority = currentPriority_;
Expand All @@ -201,7 +200,7 @@ void RuntimeScheduler_Modern::startWorkLoop(

std::shared_ptr<Task> RuntimeScheduler_Modern::selectTask(
RuntimeSchedulerTimePoint currentTime,
bool onlyExpired) const {
bool onlyExpired) {
// We need a unique lock here because we'll also remove executed tasks from
// the top of the queue.
std::unique_lock lock(schedulingMutex_);
Expand Down Expand Up @@ -229,7 +228,7 @@ std::shared_ptr<Task> RuntimeScheduler_Modern::selectTask(
void RuntimeScheduler_Modern::executeTask(
jsi::Runtime& runtime,
const std::shared_ptr<Task>& task,
RuntimeSchedulerTimePoint currentTime) const {
RuntimeSchedulerTimePoint currentTime) {
auto didUserCallbackTimeout = task->expirationTime <= currentTime;

SystraceSection s(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ class RuntimeScheduler_Modern final : public RuntimeSchedulerBase {
* To be removed when we finish testing this implementation.
* All callers should use scheduleTask with the right priority afte that.
*/
void scheduleWork(RawCallback&& callback) const noexcept override;
void scheduleWork(RawCallback&& callback) noexcept override;

/*
* Grants access to the runtime synchronously on the caller's thread.
Expand All @@ -60,15 +60,15 @@ class RuntimeScheduler_Modern final : public RuntimeSchedulerBase {
*/
std::shared_ptr<Task> scheduleTask(
SchedulerPriority priority,
jsi::Function&& callback) const noexcept override;
jsi::Function&& callback) noexcept override;

/*
* Adds a custom callback to the priority queue with the given priority.
* Triggers workloop if needed.
*/
std::shared_ptr<Task> scheduleTask(
SchedulerPriority priority,
RawCallback&& callback) const noexcept override;
RawCallback&& callback) noexcept override;

/*
* Cancelled task will never be executed.
Expand Down Expand Up @@ -122,34 +122,34 @@ class RuntimeScheduler_Modern final : public RuntimeSchedulerBase {
void callExpiredTasks(jsi::Runtime& runtime) override;

private:
mutable std::atomic<uint_fast8_t> syncTaskRequests_{0};
std::atomic<uint_fast8_t> syncTaskRequests_{0};

mutable std::priority_queue<
std::priority_queue<
std::shared_ptr<Task>,
std::vector<std::shared_ptr<Task>>,
TaskPriorityComparer>
taskQueue_;

mutable std::shared_ptr<Task> currentTask_;
std::shared_ptr<Task> currentTask_;

/**
* This protects the access to `taskQueue_` and `isWorkLoopScheduled_`.
*/
mutable std::shared_mutex schedulingMutex_;

const RuntimeExecutor runtimeExecutor_;
mutable SchedulerPriority currentPriority_{SchedulerPriority::NormalPriority};
SchedulerPriority currentPriority_{SchedulerPriority::NormalPriority};

mutable std::atomic_bool isSynchronous_{false};
std::atomic_bool isSynchronous_{false};

void scheduleWorkLoop() const;
void startWorkLoop(jsi::Runtime& runtime, bool onlyExpired) const;
void scheduleWorkLoop();
void startWorkLoop(jsi::Runtime& runtime, bool onlyExpired);

std::shared_ptr<Task> selectTask(
RuntimeSchedulerTimePoint currentTime,
bool onlyExpired) const;
bool onlyExpired);

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

/**
* Follows all the steps necessary to execute the given task (in the future,
Expand All @@ -158,7 +158,7 @@ class RuntimeScheduler_Modern final : public RuntimeSchedulerBase {
void executeTask(
jsi::Runtime& runtime,
const std::shared_ptr<Task>& task,
RuntimeSchedulerTimePoint currentTime) const;
RuntimeSchedulerTimePoint currentTime);

/*
* Returns a time point representing the current point in time. May be called
Expand All @@ -170,7 +170,7 @@ class RuntimeScheduler_Modern final : public RuntimeSchedulerBase {
* Flag indicating if callback on JavaScript queue has been
* scheduled.
*/
mutable bool isWorkLoopScheduled_{false};
bool isWorkLoopScheduled_{false};
};

} // namespace facebook::react

0 comments on commit 87dbe44

Please sign in to comment.