Commit a05b0a5c authored by Etienne Pierre-Doray's avatar Etienne Pierre-Doray Committed by Commit Bot

[TaskScheduler]: Avoid extra lock for ScheduleAdjustMaxTasksIfNeeded.

This CL splits ScheduleAdjustMaxTasksIfNeeded() into:
 - ShouldScheduleAdjustMaxTasksLockRequired()
 - ScheduleAdjustMaxTasks()
To reuse taken lock and avoid reacquiring the same locks multiple times.

Change-Id: I55e2f8fa1063e2cb5537ca14a96cb118b388047d
Reviewed-on: https://chromium-review.googlesource.com/c/1357304
Commit-Queue: Etienne Pierre-Doray <etiennep@chromium.org>
Reviewed-by: default avatarFrançois Doray <fdoray@chromium.org>
Cr-Commit-Position: refs/heads/master@{#613309}
parent 3bb44fe7
...@@ -800,6 +800,7 @@ void SchedulerWorkerPoolImpl::SchedulerWorkerDelegateImpl::BlockingEnded() { ...@@ -800,6 +800,7 @@ void SchedulerWorkerPoolImpl::SchedulerWorkerDelegateImpl::BlockingEnded() {
void SchedulerWorkerPoolImpl::SchedulerWorkerDelegateImpl::MayBlockEntered() { void SchedulerWorkerPoolImpl::SchedulerWorkerDelegateImpl::MayBlockEntered() {
DCHECK_CALLED_ON_VALID_THREAD(worker_thread_checker_); DCHECK_CALLED_ON_VALID_THREAD(worker_thread_checker_);
bool must_schedule_adjust_max_tasks = false;
{ {
AutoSchedulerLock auto_lock(outer_->lock_); AutoSchedulerLock auto_lock(outer_->lock_);
...@@ -809,16 +810,19 @@ void SchedulerWorkerPoolImpl::SchedulerWorkerDelegateImpl::MayBlockEntered() { ...@@ -809,16 +810,19 @@ void SchedulerWorkerPoolImpl::SchedulerWorkerDelegateImpl::MayBlockEntered() {
++outer_->num_pending_may_block_workers_; ++outer_->num_pending_may_block_workers_;
if (is_running_best_effort_task_) if (is_running_best_effort_task_)
++outer_->num_pending_best_effort_may_block_workers_; ++outer_->num_pending_best_effort_may_block_workers_;
must_schedule_adjust_max_tasks =
outer_->MustScheduleAdjustMaxTasksLockRequired();
} }
outer_->ScheduleAdjustMaxTasksIfNeeded(); if (must_schedule_adjust_max_tasks)
outer_->ScheduleAdjustMaxTasks();
} }
void SchedulerWorkerPoolImpl::SchedulerWorkerDelegateImpl::WillBlockEntered() { void SchedulerWorkerPoolImpl::SchedulerWorkerDelegateImpl::WillBlockEntered() {
DCHECK_CALLED_ON_VALID_THREAD(worker_thread_checker_); DCHECK_CALLED_ON_VALID_THREAD(worker_thread_checker_);
bool wake_up_allowed = false; bool must_schedule_adjust_max_tasks = false;
SchedulerWorkerStarter starter(outer_); SchedulerWorkerStarter starter(outer_);
{ {
std::unique_ptr<PriorityQueue::Transaction> transaction( std::unique_ptr<PriorityQueue::Transaction> transaction(
outer_->shared_priority_queue_.BeginTransaction()); outer_->shared_priority_queue_.BeginTransaction());
...@@ -843,11 +847,14 @@ void SchedulerWorkerPoolImpl::SchedulerWorkerDelegateImpl::WillBlockEntered() { ...@@ -843,11 +847,14 @@ void SchedulerWorkerPoolImpl::SchedulerWorkerDelegateImpl::WillBlockEntered() {
// ScopedBlockingCalls in parallel and we had work on the PQ. // ScopedBlockingCalls in parallel and we had work on the PQ.
starter.ScheduleStart(outer_->WakeUpOneWorkerLockRequired()); starter.ScheduleStart(outer_->WakeUpOneWorkerLockRequired());
} }
must_schedule_adjust_max_tasks =
outer_->MustScheduleAdjustMaxTasksLockRequired();
} }
// TODO(crbug.com/813857): This can be better handled in the PostTask() // TODO(crbug.com/813857): This can be better handled in the PostTask()
// codepath. We really only should do this if there are tasks pending. // codepath. We really only should do this if there are tasks pending.
if (wake_up_allowed) if (must_schedule_adjust_max_tasks)
outer_->ScheduleAdjustMaxTasksIfNeeded(); outer_->ScheduleAdjustMaxTasks();
} }
bool SchedulerWorkerPoolImpl::SchedulerWorkerDelegateImpl:: bool SchedulerWorkerPoolImpl::SchedulerWorkerDelegateImpl::
...@@ -908,12 +915,15 @@ SchedulerWorkerPoolImpl::WakeUpOneWorkerLockRequired() { ...@@ -908,12 +915,15 @@ SchedulerWorkerPoolImpl::WakeUpOneWorkerLockRequired() {
} }
void SchedulerWorkerPoolImpl::WakeUpOneWorker() { void SchedulerWorkerPoolImpl::WakeUpOneWorker() {
bool must_schedule_adjust_max_tasks = false;
SchedulerWorkerStarter starter(tracked_ref_factory_.GetTrackedRef()); SchedulerWorkerStarter starter(tracked_ref_factory_.GetTrackedRef());
{ {
AutoSchedulerLock auto_lock(lock_); AutoSchedulerLock auto_lock(lock_);
starter.ScheduleStart(WakeUpOneWorkerLockRequired()); starter.ScheduleStart(WakeUpOneWorkerLockRequired());
must_schedule_adjust_max_tasks = MustScheduleAdjustMaxTasksLockRequired();
} }
ScheduleAdjustMaxTasksIfNeeded(); if (must_schedule_adjust_max_tasks)
ScheduleAdjustMaxTasks();
} }
scoped_refptr<SchedulerWorker> scoped_refptr<SchedulerWorker>
...@@ -1015,7 +1025,7 @@ void SchedulerWorkerPoolImpl::AdjustMaxTasks() { ...@@ -1015,7 +1025,7 @@ void SchedulerWorkerPoolImpl::AdjustMaxTasks() {
std::min(max_tasks_ - previous_max_tasks, num_pending_sequences); std::min(max_tasks_ - previous_max_tasks, num_pending_sequences);
for (size_t i = 0; i < num_wake_ups_needed; ++i) { for (size_t i = 0; i < num_wake_ups_needed; ++i) {
// No need to call ScheduleAdjustMaxTasksIfNeeded() as the caller will // No need to call ScheduleAdjustMaxTasks() as the caller will
// take care of that for us. // take care of that for us.
starter.ScheduleStart(WakeUpOneWorkerLockRequired()); starter.ScheduleStart(WakeUpOneWorkerLockRequired());
} }
...@@ -1030,14 +1040,8 @@ TimeDelta SchedulerWorkerPoolImpl::MayBlockThreshold() const { ...@@ -1030,14 +1040,8 @@ TimeDelta SchedulerWorkerPoolImpl::MayBlockThreshold() const {
return may_block_threshold_; return may_block_threshold_;
} }
void SchedulerWorkerPoolImpl::ScheduleAdjustMaxTasksIfNeeded() { void SchedulerWorkerPoolImpl::ScheduleAdjustMaxTasks() {
{ DCHECK(polling_max_tasks_);
AutoSchedulerLock auto_lock(lock_);
if (polling_max_tasks_ || !ShouldPeriodicallyAdjustMaxTasksLockRequired()) {
return;
}
polling_max_tasks_ = true;
}
service_thread_task_runner_->PostDelayedTask( service_thread_task_runner_->PostDelayedTask(
FROM_HERE, FROM_HERE,
BindOnce(&SchedulerWorkerPoolImpl::AdjustMaxTasksFunction, BindOnce(&SchedulerWorkerPoolImpl::AdjustMaxTasksFunction,
...@@ -1045,6 +1049,13 @@ void SchedulerWorkerPoolImpl::ScheduleAdjustMaxTasksIfNeeded() { ...@@ -1045,6 +1049,13 @@ void SchedulerWorkerPoolImpl::ScheduleAdjustMaxTasksIfNeeded() {
blocked_workers_poll_period_); blocked_workers_poll_period_);
} }
bool SchedulerWorkerPoolImpl::MustScheduleAdjustMaxTasksLockRequired() {
if (polling_max_tasks_ || !ShouldPeriodicallyAdjustMaxTasksLockRequired())
return false;
polling_max_tasks_ = true;
return true;
}
void SchedulerWorkerPoolImpl::AdjustMaxTasksFunction() { void SchedulerWorkerPoolImpl::AdjustMaxTasksFunction() {
DCHECK(service_thread_task_runner_->RunsTasksInCurrentSequence()); DCHECK(service_thread_task_runner_->RunsTasksInCurrentSequence());
...@@ -1058,11 +1069,7 @@ void SchedulerWorkerPoolImpl::AdjustMaxTasksFunction() { ...@@ -1058,11 +1069,7 @@ void SchedulerWorkerPoolImpl::AdjustMaxTasksFunction() {
return; return;
} }
} }
service_thread_task_runner_->PostDelayedTask( ScheduleAdjustMaxTasks();
FROM_HERE,
BindOnce(&SchedulerWorkerPoolImpl::AdjustMaxTasksFunction,
Unretained(this)),
blocked_workers_poll_period_);
} }
bool SchedulerWorkerPoolImpl::ShouldPeriodicallyAdjustMaxTasksLockRequired() { bool SchedulerWorkerPoolImpl::ShouldPeriodicallyAdjustMaxTasksLockRequired() {
......
...@@ -227,8 +227,11 @@ class BASE_EXPORT SchedulerWorkerPoolImpl : public SchedulerWorkerPool { ...@@ -227,8 +227,11 @@ class BASE_EXPORT SchedulerWorkerPoolImpl : public SchedulerWorkerPool {
TimeDelta MayBlockThreshold() const; TimeDelta MayBlockThreshold() const;
// Starts calling AdjustMaxTasks() periodically on // Starts calling AdjustMaxTasks() periodically on
// |service_thread_task_runner_| if not already requested. // |service_thread_task_runner_|.
void ScheduleAdjustMaxTasksIfNeeded(); void ScheduleAdjustMaxTasks();
// Returns true if ScheduleAdjustMaxTasks() must be called.
bool MustScheduleAdjustMaxTasksLockRequired();
// Calls AdjustMaxTasks() and schedules it again as necessary. May only be // Calls AdjustMaxTasks() and schedules it again as necessary. May only be
// called from the service thread. // called from the service thread.
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment