Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(18)

Side by Side Diff: base/task_scheduler/sequence.cc

Issue 2164103002: TaskScheduler: Don't delete Tasks in the scope of a Sequence's lock. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: CR robliao #6 (initialize variable at declaration) Created 4 years, 5 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View unified diff | Download patch
« no previous file with comments | « no previous file | base/task_scheduler/sequence_unittest.cc » ('j') | no next file with comments »
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
OLDNEW
1 // Copyright 2016 The Chromium Authors. All rights reserved. 1 // Copyright 2016 The Chromium Authors. All rights reserved.
2 // Use of this source code is governed by a BSD-style license that can be 2 // Use of this source code is governed by a BSD-style license that can be
3 // found in the LICENSE file. 3 // found in the LICENSE file.
4 4
5 #include "base/task_scheduler/sequence.h" 5 #include "base/task_scheduler/sequence.h"
6 6
7 #include <utility> 7 #include <utility>
8 8
9 #include "base/logging.h" 9 #include "base/logging.h"
10 #include "base/time/time.h" 10 #include "base/time/time.h"
(...skipping 18 matching lines...) Expand all
29 const Task* Sequence::PeekTask() const { 29 const Task* Sequence::PeekTask() const {
30 AutoSchedulerLock auto_lock(lock_); 30 AutoSchedulerLock auto_lock(lock_);
31 31
32 if (queue_.empty()) 32 if (queue_.empty())
33 return nullptr; 33 return nullptr;
34 34
35 return queue_.front().get(); 35 return queue_.front().get();
36 } 36 }
37 37
38 bool Sequence::PopTask() { 38 bool Sequence::PopTask() {
39 AutoSchedulerLock auto_lock(lock_); 39 // Delete the popped task outside the scope of |lock_|. This prevents a double
40 DCHECK(!queue_.empty()); 40 // acquisition of |lock_| if the task's destructor tries to post a task to
41 // this Sequence and reduces contention.
42 std::unique_ptr<Task> delete_outside_lock_scope;
43 bool sequence_empty_after_pop = false;
41 44
42 const int priority_index = 45 {
43 static_cast<int>(queue_.front()->traits.priority()); 46 AutoSchedulerLock auto_lock(lock_);
44 DCHECK_GT(num_tasks_per_priority_[priority_index], 0U); 47 DCHECK(!queue_.empty());
45 --num_tasks_per_priority_[priority_index];
46 48
47 queue_.pop(); 49 const int priority_index =
48 return queue_.empty(); 50 static_cast<int>(queue_.front()->traits.priority());
51 DCHECK_GT(num_tasks_per_priority_[priority_index], 0U);
52 --num_tasks_per_priority_[priority_index];
53
54 delete_outside_lock_scope = std::move(queue_.front());
55 queue_.pop();
56 sequence_empty_after_pop = queue_.empty();
57 }
58
59 return sequence_empty_after_pop;
49 } 60 }
50 61
51 SequenceSortKey Sequence::GetSortKey() const { 62 SequenceSortKey Sequence::GetSortKey() const {
52 TaskPriority priority = TaskPriority::LOWEST; 63 TaskPriority priority = TaskPriority::LOWEST;
53 base::TimeTicks next_task_sequenced_time; 64 base::TimeTicks next_task_sequenced_time;
54 65
55 { 66 {
56 AutoSchedulerLock auto_lock(lock_); 67 AutoSchedulerLock auto_lock(lock_);
57 DCHECK(!queue_.empty()); 68 DCHECK(!queue_.empty());
58 69
(...skipping 11 matching lines...) Expand all
70 next_task_sequenced_time = queue_.front()->sequenced_time; 81 next_task_sequenced_time = queue_.front()->sequenced_time;
71 } 82 }
72 83
73 return SequenceSortKey(priority, next_task_sequenced_time); 84 return SequenceSortKey(priority, next_task_sequenced_time);
74 } 85 }
75 86
76 Sequence::~Sequence() = default; 87 Sequence::~Sequence() = default;
77 88
78 } // namespace internal 89 } // namespace internal
79 } // namespace base 90 } // namespace base
OLDNEW
« no previous file with comments | « no previous file | base/task_scheduler/sequence_unittest.cc » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698