Commit 17b154f7 authored by Etienne Pierre-doray's avatar Etienne Pierre-doray Committed by Commit Bot

[ThreadPool]: Use const ref in RemoveTaskSource.

Change-Id: I3b4c51879cef4b97c777715f5d190cb33decbd77
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1834897
Commit-Queue: Etienne Pierre-Doray <etiennep@chromium.org>
Reviewed-by: default avatarGabriel Charette <gab@chromium.org>
Cr-Commit-Position: refs/heads/master@{#706570}
parent 83f295d5
...@@ -139,20 +139,18 @@ RegisteredTaskSource PriorityQueue::PopTaskSource() { ...@@ -139,20 +139,18 @@ RegisteredTaskSource PriorityQueue::PopTaskSource() {
} }
RegisteredTaskSource PriorityQueue::RemoveTaskSource( RegisteredTaskSource PriorityQueue::RemoveTaskSource(
scoped_refptr<TaskSource> task_source) { const TaskSource& task_source) {
DCHECK(task_source);
if (IsEmpty()) if (IsEmpty())
return nullptr; return nullptr;
const HeapHandle heap_handle = task_source->heap_handle(); const HeapHandle heap_handle = task_source.heap_handle();
if (!heap_handle.IsValid()) if (!heap_handle.IsValid())
return nullptr; return nullptr;
TaskSourceAndSortKey& task_source_and_sort_key = TaskSourceAndSortKey& task_source_and_sort_key =
const_cast<PriorityQueue::TaskSourceAndSortKey&>( const_cast<PriorityQueue::TaskSourceAndSortKey&>(
container_.at(heap_handle)); container_.at(heap_handle));
DCHECK_EQ(task_source_and_sort_key.task_source().get(), task_source); DCHECK_EQ(task_source_and_sort_key.task_source().get(), &task_source);
RegisteredTaskSource registered_task_source = RegisteredTaskSource registered_task_source =
task_source_and_sort_key.take_task_source(); task_source_and_sort_key.take_task_source();
......
...@@ -49,7 +49,7 @@ class BASE_EXPORT PriorityQueue { ...@@ -49,7 +49,7 @@ class BASE_EXPORT PriorityQueue {
// RegisteredTaskSource which evaluates to true if successful, or false if // RegisteredTaskSource which evaluates to true if successful, or false if
// |task_source| is not currently in the PriorityQueue or the PriorityQueue is // |task_source| is not currently in the PriorityQueue or the PriorityQueue is
// empty. // empty.
RegisteredTaskSource RemoveTaskSource(scoped_refptr<TaskSource> task_source); RegisteredTaskSource RemoveTaskSource(const TaskSource& task_source);
// Updates the sort key of the TaskSource in |transaction| to // Updates the sort key of the TaskSource in |transaction| to
// match its current traits. No-ops if the TaskSource is not in the // match its current traits. No-ops if the TaskSource is not in the
......
...@@ -144,34 +144,34 @@ TEST_F(PriorityQueueWithSequencesTest, RemoveSequence) { ...@@ -144,34 +144,34 @@ TEST_F(PriorityQueueWithSequencesTest, RemoveSequence) {
// Remove |sequence_a| from the PriorityQueue. |sequence_b| is still the // Remove |sequence_a| from the PriorityQueue. |sequence_b| is still the
// sequence with the highest priority. // sequence with the highest priority.
EXPECT_TRUE(pq.RemoveTaskSource(sequence_a).Unregister()); EXPECT_TRUE(pq.RemoveTaskSource(*sequence_a).Unregister());
EXPECT_EQ(sort_key_b, pq.PeekSortKey()); EXPECT_EQ(sort_key_b, pq.PeekSortKey());
ExpectNumSequences(1U, 0U, 2U); ExpectNumSequences(1U, 0U, 2U);
// RemoveTaskSource() should return false if called on a sequence not in the // RemoveTaskSource() should return false if called on a sequence not in the
// PriorityQueue. // PriorityQueue.
EXPECT_FALSE(pq.RemoveTaskSource(sequence_a).Unregister()); EXPECT_FALSE(pq.RemoveTaskSource(*sequence_a).Unregister());
ExpectNumSequences(1U, 0U, 2U); ExpectNumSequences(1U, 0U, 2U);
// Remove |sequence_b| from the PriorityQueue. |sequence_c| becomes the // Remove |sequence_b| from the PriorityQueue. |sequence_c| becomes the
// sequence with the highest priority. // sequence with the highest priority.
EXPECT_TRUE(pq.RemoveTaskSource(sequence_b).Unregister()); EXPECT_TRUE(pq.RemoveTaskSource(*sequence_b).Unregister());
EXPECT_EQ(sort_key_c, pq.PeekSortKey()); EXPECT_EQ(sort_key_c, pq.PeekSortKey());
ExpectNumSequences(1U, 0U, 1U); ExpectNumSequences(1U, 0U, 1U);
// Remove |sequence_d| from the PriorityQueue. |sequence_c| is still the // Remove |sequence_d| from the PriorityQueue. |sequence_c| is still the
// sequence with the highest priority. // sequence with the highest priority.
EXPECT_TRUE(pq.RemoveTaskSource(sequence_d).Unregister()); EXPECT_TRUE(pq.RemoveTaskSource(*sequence_d).Unregister());
EXPECT_EQ(sort_key_c, pq.PeekSortKey()); EXPECT_EQ(sort_key_c, pq.PeekSortKey());
ExpectNumSequences(0U, 0U, 1U); ExpectNumSequences(0U, 0U, 1U);
// Remove |sequence_c| from the PriorityQueue, making it empty. // Remove |sequence_c| from the PriorityQueue, making it empty.
EXPECT_TRUE(pq.RemoveTaskSource(sequence_c).Unregister()); EXPECT_TRUE(pq.RemoveTaskSource(*sequence_c).Unregister());
EXPECT_TRUE(pq.IsEmpty()); EXPECT_TRUE(pq.IsEmpty());
ExpectNumSequences(0U, 0U, 0U); ExpectNumSequences(0U, 0U, 0U);
// Return false if RemoveTaskSource() is called on an empty PriorityQueue. // Return false if RemoveTaskSource() is called on an empty PriorityQueue.
EXPECT_FALSE(pq.RemoveTaskSource(sequence_c).Unregister()); EXPECT_FALSE(pq.RemoveTaskSource(*sequence_c).Unregister());
ExpectNumSequences(0U, 0U, 0U); ExpectNumSequences(0U, 0U, 0U);
} }
......
...@@ -241,7 +241,7 @@ bool MockPooledTaskRunnerDelegate::EnqueueJobTaskSource( ...@@ -241,7 +241,7 @@ bool MockPooledTaskRunnerDelegate::EnqueueJobTaskSource(
void MockPooledTaskRunnerDelegate::RemoveJobTaskSource( void MockPooledTaskRunnerDelegate::RemoveJobTaskSource(
scoped_refptr<JobTaskSource> task_source) { scoped_refptr<JobTaskSource> task_source) {
thread_group_->RemoveTaskSource(std::move(task_source)); thread_group_->RemoveTaskSource(*task_source);
} }
bool MockPooledTaskRunnerDelegate::IsRunningPoolWithTraits( bool MockPooledTaskRunnerDelegate::IsRunningPoolWithTraits(
......
...@@ -139,9 +139,9 @@ ThreadGroup::GetNumAdditionalWorkersForForegroundTaskSourcesLockRequired() ...@@ -139,9 +139,9 @@ ThreadGroup::GetNumAdditionalWorkersForForegroundTaskSourcesLockRequired()
} }
RegisteredTaskSource ThreadGroup::RemoveTaskSource( RegisteredTaskSource ThreadGroup::RemoveTaskSource(
scoped_refptr<TaskSource> task_source) { const TaskSource& task_source) {
CheckedAutoLock auto_lock(lock_); CheckedAutoLock auto_lock(lock_);
return priority_queue_.RemoveTaskSource(std::move(task_source)); return priority_queue_.RemoveTaskSource(task_source);
} }
void ThreadGroup::ReEnqueueTaskSourceLockRequired( void ThreadGroup::ReEnqueueTaskSourceLockRequired(
......
...@@ -65,7 +65,7 @@ class BASE_EXPORT ThreadGroup { ...@@ -65,7 +65,7 @@ class BASE_EXPORT ThreadGroup {
// RegisteredTaskSource that evaluats to true if successful, or false if // RegisteredTaskSource that evaluats to true if successful, or false if
// |task_source| is not currently in |priority_queue_|, such as when a worker // |task_source| is not currently in |priority_queue_|, such as when a worker
// is running a task from it. // is running a task from it.
RegisteredTaskSource RemoveTaskSource(scoped_refptr<TaskSource> task_source); RegisteredTaskSource RemoveTaskSource(const TaskSource& task_source);
// Updates the position of the TaskSource in |transaction| in this // Updates the position of the TaskSource in |transaction| in this
// ThreadGroup's PriorityQueue based on the TaskSource's current traits. // ThreadGroup's PriorityQueue based on the TaskSource's current traits.
......
...@@ -431,7 +431,7 @@ void ThreadPoolImpl::RemoveJobTaskSource( ...@@ -431,7 +431,7 @@ void ThreadPoolImpl::RemoveJobTaskSource(
auto transaction = task_source->BeginTransaction(); auto transaction = task_source->BeginTransaction();
ThreadGroup* const current_thread_group = ThreadGroup* const current_thread_group =
GetThreadGroupForTraits(transaction.traits()); GetThreadGroupForTraits(transaction.traits());
current_thread_group->RemoveTaskSource(std::move(task_source)); current_thread_group->RemoveTaskSource(*task_source);
} }
bool ThreadPoolImpl::IsRunningPoolWithTraits(const TaskTraits& traits) const { bool ThreadPoolImpl::IsRunningPoolWithTraits(const TaskTraits& traits) const {
...@@ -466,7 +466,7 @@ void ThreadPoolImpl::UpdatePriority(scoped_refptr<TaskSource> task_source, ...@@ -466,7 +466,7 @@ void ThreadPoolImpl::UpdatePriority(scoped_refptr<TaskSource> task_source,
// |task_source| is changing thread groups; remove it from its current // |task_source| is changing thread groups; remove it from its current
// thread group and reenqueue it. // thread group and reenqueue it.
auto registered_task_source = auto registered_task_source =
current_thread_group->RemoveTaskSource(task_source); current_thread_group->RemoveTaskSource(*task_source);
if (registered_task_source) { if (registered_task_source) {
DCHECK(task_source); DCHECK(task_source);
new_thread_group->PushTaskSourceAndWakeUpWorkers( new_thread_group->PushTaskSourceAndWakeUpWorkers(
......
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