Skip to content

Commit

Permalink
Replace base::Callback with base::OnceCallback in base::PendingTask
Browse files Browse the repository at this point in the history
After this CL, PendingTask holds the posted task as OnceClosure. And to
do that, PendingTasks get non-const on a task invocation, due to the
mutation the posted task for the consumption by the task invocation.

BUG=554299

Review-Url: https://codereview.chromium.org/2386653002
Cr-Commit-Position: refs/heads/master@{#425318}
  • Loading branch information
tzik authored and Commit bot committed Oct 14, 2016
1 parent d2e069c commit 739ffe3
Show file tree
Hide file tree
Showing 13 changed files with 50 additions and 41 deletions.
24 changes: 11 additions & 13 deletions base/debug/task_annotator.cc
Original file line number Diff line number Diff line change
Expand Up @@ -28,34 +28,32 @@ void TaskAnnotator::DidQueueTask(const char* queue_function,
}

void TaskAnnotator::RunTask(const char* queue_function,
const PendingTask& pending_task) {
ScopedTaskRunActivity task_activity(pending_task);
PendingTask* pending_task) {
ScopedTaskRunActivity task_activity(*pending_task);

tracked_objects::TaskStopwatch stopwatch;
stopwatch.Start();
tracked_objects::Duration queue_duration =
stopwatch.StartTime() - pending_task.EffectiveTimePosted();
stopwatch.StartTime() - pending_task->EffectiveTimePosted();

TRACE_EVENT_WITH_FLOW1(TRACE_DISABLED_BY_DEFAULT("toplevel.flow"),
queue_function,
TRACE_ID_MANGLE(GetTaskTraceID(pending_task)),
TRACE_EVENT_FLAG_FLOW_IN,
"queue_duration",
queue_duration.InMilliseconds());
TRACE_EVENT_WITH_FLOW1(
TRACE_DISABLED_BY_DEFAULT("toplevel.flow"), queue_function,
TRACE_ID_MANGLE(GetTaskTraceID(*pending_task)), TRACE_EVENT_FLAG_FLOW_IN,
"queue_duration", queue_duration.InMilliseconds());

// Before running the task, store the program counter where it was posted
// and deliberately alias it to ensure it is on the stack if the task
// crashes. Be careful not to assume that the variable itself will have the
// expected value when displayed by the optimizer in an optimized build.
// Look at a memory dump of the stack.
const void* program_counter = pending_task.posted_from.program_counter();
const void* program_counter = pending_task->posted_from.program_counter();
debug::Alias(&program_counter);

pending_task.task.Run();
std::move(pending_task->task).Run();

stopwatch.Stop();
tracked_objects::ThreadData::TallyRunOnNamedThreadIfTracking(
pending_task, stopwatch);
tracked_objects::ThreadData::TallyRunOnNamedThreadIfTracking(*pending_task,
stopwatch);
}

uint64_t TaskAnnotator::GetTaskTraceID(const PendingTask& task) const {
Expand Down
2 changes: 1 addition & 1 deletion base/debug/task_annotator.h
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ class BASE_EXPORT TaskAnnotator {

// Run a previously queued task. |queue_function| should match what was
// passed into |DidQueueTask| for this task.
void RunTask(const char* queue_function, const PendingTask& pending_task);
void RunTask(const char* queue_function, PendingTask* pending_task);

private:
// Creates a process-wide unique ID to represent this task in trace events.
Expand Down
2 changes: 1 addition & 1 deletion base/debug/task_annotator_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ TEST(TaskAnnotatorTest, QueueAndRunTask) {
TaskAnnotator annotator;
annotator.DidQueueTask("TaskAnnotatorTest::Queue", pending_task);
EXPECT_EQ(0, result);
annotator.RunTask("TaskAnnotatorTest::Queue", pending_task);
annotator.RunTask("TaskAnnotatorTest::Queue", &pending_task);
EXPECT_EQ(123, result);
}

Expand Down
14 changes: 7 additions & 7 deletions base/message_loop/message_loop.cc
Original file line number Diff line number Diff line change
Expand Up @@ -387,15 +387,15 @@ bool MessageLoop::ProcessNextDelayedNonNestableTask() {
std::move(deferred_non_nestable_work_queue_.front());
deferred_non_nestable_work_queue_.pop();

RunTask(pending_task);
RunTask(&pending_task);
return true;
}

void MessageLoop::RunTask(const PendingTask& pending_task) {
void MessageLoop::RunTask(PendingTask* pending_task) {
DCHECK(nestable_tasks_allowed_);

#if defined(OS_WIN)
if (pending_task.is_high_res) {
if (pending_task->is_high_res) {
pending_high_res_tasks_--;
CHECK_GE(pending_high_res_tasks_, 0);
}
Expand All @@ -404,20 +404,20 @@ void MessageLoop::RunTask(const PendingTask& pending_task) {
// Execute the task and assume the worst: It is probably not reentrant.
nestable_tasks_allowed_ = false;

TRACE_TASK_EXECUTION("MessageLoop::RunTask", pending_task);
TRACE_TASK_EXECUTION("MessageLoop::RunTask", *pending_task);

FOR_EACH_OBSERVER(TaskObserver, task_observers_,
WillProcessTask(pending_task));
WillProcessTask(*pending_task));
task_annotator_.RunTask("MessageLoop::PostTask", pending_task);
FOR_EACH_OBSERVER(TaskObserver, task_observers_,
DidProcessTask(pending_task));
DidProcessTask(*pending_task));

nestable_tasks_allowed_ = true;
}

bool MessageLoop::DeferOrRunPendingTask(PendingTask pending_task) {
if (pending_task.nestable || run_loop_->run_depth_ == 1) {
RunTask(pending_task);
RunTask(&pending_task);
// Show that we ran a task (Note: a new one might arrive as a
// consequence!).
return true;
Expand Down
2 changes: 1 addition & 1 deletion base/message_loop/message_loop.h
Original file line number Diff line number Diff line change
Expand Up @@ -314,7 +314,7 @@ class BASE_EXPORT MessageLoop : public MessagePump::Delegate {
debug::TaskAnnotator* task_annotator() { return &task_annotator_; }

// Runs the specified PendingTask.
void RunTask(const PendingTask& pending_task);
void RunTask(PendingTask* pending_task);

// Disallow nesting. After this is called, running a nested RunLoop or calling
// Add/RemoveNestingObserver() on this MessageLoop will crash.
Expand Down
2 changes: 1 addition & 1 deletion base/message_loop/message_pump_perftest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -272,7 +272,7 @@ class PostTaskTest : public testing::Test {
while (!loop_local_queue.empty()) {
PendingTask t = std::move(loop_local_queue.front());
loop_local_queue.pop();
loop.RunTask(t);
loop.RunTask(&t);
}
}

Expand Down
10 changes: 4 additions & 6 deletions base/pending_task.cc
Original file line number Diff line number Diff line change
Expand Up @@ -9,26 +9,24 @@
namespace base {

PendingTask::PendingTask(const tracked_objects::Location& posted_from,
base::Closure task)
OnceClosure task)
: base::TrackingInfo(posted_from, TimeTicks()),
task(std::move(task)),
posted_from(posted_from),
sequence_num(0),
nestable(true),
is_high_res(false) {
}
is_high_res(false) {}

PendingTask::PendingTask(const tracked_objects::Location& posted_from,
base::Closure task,
OnceClosure task,
TimeTicks delayed_run_time,
bool nestable)
: base::TrackingInfo(posted_from, delayed_run_time),
task(std::move(task)),
posted_from(posted_from),
sequence_num(0),
nestable(nestable),
is_high_res(false) {
}
is_high_res(false) {}

PendingTask::PendingTask(PendingTask&& other) = default;

Expand Down
7 changes: 3 additions & 4 deletions base/pending_task.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,9 @@ namespace base {
// Contains data about a pending task. Stored in TaskQueue and DelayedTaskQueue
// for use by classes that queue and execute tasks.
struct BASE_EXPORT PendingTask : public TrackingInfo {
PendingTask(const tracked_objects::Location& posted_from, OnceClosure task);
PendingTask(const tracked_objects::Location& posted_from,
Closure task);
PendingTask(const tracked_objects::Location& posted_from,
Closure task,
OnceClosure task,
TimeTicks delayed_run_time,
bool nestable);
PendingTask(PendingTask&& other);
Expand All @@ -33,7 +32,7 @@ struct BASE_EXPORT PendingTask : public TrackingInfo {
bool operator<(const PendingTask& other) const;

// The task to run.
Closure task;
OnceClosure task;

// The site this PendingTask was posted from.
tracked_objects::Location posted_from;
Expand Down
2 changes: 1 addition & 1 deletion base/task_scheduler/task_tracker.cc
Original file line number Diff line number Diff line change
Expand Up @@ -255,7 +255,7 @@ bool TaskTracker::RunTask(std::unique_ptr<Task> task,
sequence_token));

debug::TaskAnnotator task_annotator;
task_annotator.RunTask(kQueueFunctionName, *task);
task_annotator.RunTask(kQueueFunctionName, task.get());
}

AfterRunTask(shutdown_behavior);
Expand Down
2 changes: 1 addition & 1 deletion base/threading/worker_pool_posix.cc
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ void WorkerThread::ThreadMain() {

tracked_objects::TaskStopwatch stopwatch;
stopwatch.Start();
pending_task.task.Run();
std::move(pending_task.task).Run();
stopwatch.Stop();

tracked_objects::ThreadData::TallyRunOnWorkerThreadIfTracking(
Expand Down
2 changes: 1 addition & 1 deletion base/threading/worker_pool_win.cc
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ DWORD CALLBACK WorkItemCallback(void* param) {

tracked_objects::TaskStopwatch stopwatch;
stopwatch.Start();
pending_task->task.Run();
std::move(pending_task->task).Run();
stopwatch.Stop();

g_worker_pool_running_on_this_thread.Get().Set(false);
Expand Down
3 changes: 2 additions & 1 deletion components/timers/alarm_timer_chromeos.cc
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,8 @@ void AlarmTimer::OnTimerFired() {

// Run the task.
TRACE_TASK_EXECUTION("AlarmTimer::OnTimerFired", *pending_user_task);
base::debug::TaskAnnotator().RunTask("AlarmTimer::Reset", *pending_user_task);
base::debug::TaskAnnotator().RunTask("AlarmTimer::Reset",
pending_user_task.get());

// If the timer wasn't deleted, stopped or reset by the callback, reset or
// stop it.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,16 @@ void RecordImmediateTaskQueueingDuration(tracked_objects::Duration duration) {
double MonotonicTimeInSeconds(base::TimeTicks timeTicks) {
return (timeTicks - base::TimeTicks()).InSecondsF();
}

// Converts a OnceClosure to a RepeatingClosure. It hits CHECK failure to run
// the resulting RepeatingClosure more than once.
// TODO(tzik): This will be unneeded after the Closure-to-OnceClosure migration
// on TaskRunner finished. Remove it once it gets unneeded.
base::RepeatingClosure UnsafeConvertOnceClosureToRepeating(
base::OnceClosure cb) {
return base::BindRepeating([](base::OnceClosure cb) { std::move(cb).Run(); },
base::Passed(&cb));
}
}

TaskQueueManager::TaskQueueManager(
Expand Down Expand Up @@ -319,8 +329,11 @@ TaskQueueManager::ProcessTaskResult TaskQueueManager::ProcessTaskFromWorkQueue(
// arbitrarily delayed so the additional delay should not be a problem.
// TODO(skyostil): Figure out a way to not forget which task queue the
// task is associated with. See http://crbug.com/522843.
delegate_->PostNonNestableTask(pending_task.posted_from,
std::move(pending_task.task));
// TODO(tzik): Remove base::UnsafeConvertOnceClosureToRepeating once
// TaskRunners have migrated to OnceClosure.
delegate_->PostNonNestableTask(
pending_task.posted_from,
UnsafeConvertOnceClosureToRepeating(std::move(pending_task.task)));
return ProcessTaskResult::DEFERRED;
}

Expand All @@ -341,7 +354,7 @@ TaskQueueManager::ProcessTaskResult TaskQueueManager::ProcessTaskFromWorkQueue(
internal::TaskQueueImpl* prev_executing_task_queue =
currently_executing_task_queue_;
currently_executing_task_queue_ = queue;
task_annotator_.RunTask("TaskQueueManager::PostTask", pending_task);
task_annotator_.RunTask("TaskQueueManager::PostTask", &pending_task);
// Detect if the TaskQueueManager just got deleted. If this happens we must
// not access any member variables after this point.
if (protect->HasOneRef())
Expand Down

0 comments on commit 739ffe3

Please sign in to comment.