TaskScheduler: Set the IO allowed bit in TaskTracker::RunTask().

base/task_scheduler/task_tracker.cc

 // Copyright 2016 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "base/task_scheduler/task_tracker.h"
 
 #include <limits>
 
 #include "base/callback.h"
 #include "base/debug/task_annotator.h"
@@ skipping 201 matching lines @@
                           const SequenceToken& sequence_token) {
   DCHECK(task);
   DCHECK(sequence_token.IsValid());
 
   const TaskShutdownBehavior shutdown_behavior =
       task->traits.shutdown_behavior();
   const bool can_run_task = BeforeRunTask(shutdown_behavior);
   const bool is_delayed = !task->delayed_run_time.is_null();
 
   if (can_run_task) {
-    // All tasks run through here and the scheduler itself doesn't use

[danakj, 2016/11/28 22:55:56]

This comment is no less true now is it? So IOAllowed should be able to follow
the same pattern and just leave the thread in whatever state since the next task
on the thread will come through here? Just making sure I understand, and then
wondering why the change?

[fdoray, 2016/11/29 15:41:20]

I decided to reset ThreadRestrictions in RunTask() to avoid having singletons
and/or IO disallowed on the main thread after running TaskTracker tests.

We could also reset ThreadRestrictions from
TaskSchedulerTaskTrackerTest::TearDown(). I don't like this solution because
once we add wait restrictions:
- We would need a ScopedWaitAllowed in SchedulerWorker::WaitForWork(). Might as
well reset the state from RunTask().
- - TaskSchedulerTaskTrackerTest would need to be a friend of ThreadRestrictions
to reset the wait allowed bit.

-    // singletons. Therefore, it isn't necessary to reset the singleton allowed
-    // bit after running the task.
-    ThreadRestrictions::SetSingletonAllowed(
-        task->traits.shutdown_behavior() !=
-        TaskShutdownBehavior::CONTINUE_ON_SHUTDOWN);
+    const bool previous_singleton_allowed =
+        ThreadRestrictions::SetSingletonAllowed(
+            task->traits.shutdown_behavior() !=
+            TaskShutdownBehavior::CONTINUE_ON_SHUTDOWN);
+    const bool previous_io_allowed =
+        ThreadRestrictions::SetIOAllowed(task->traits.with_file_io());

[danakj, 2016/11/28 22:55:56]

Is reading and writing thread-local storage before every task going to show up
in profiling? Seems like since threads are owned by task scheduler it could do
something cheaper.

[fdoray, 2016/11/29 15:41:20]

I don't think we need to optimize this since ThreadRestrictions methods are
no-ops in non-DCHECK builds. In release with DCHECK builds,
SetSingletonAllowed() takes 10 ns on my Windows Z840.

  TimeTicks start = TimeTicks::Now();
  constexpr size_t kIterations = 1000000000;
  for (size_t i = 0; i < kIterations; ++i)
    ThreadRestrictions::SetSingletonAllowed(i % 2 == 0);
  TimeTicks end = TimeTicks::Now();
  LOG(ERROR) << "Elapsed: " << (end - start);

[danakj, 2016/12/01 21:59:04]

The assert methods are no-ops. The setters are not.
https://cs.chromium.org/chromium/src/base/threading/thread_restrictions.cc?rc...
While that's quick per run, hundreds of tasks run per frame in chrome, perhaps?
So thats getting into microseconds out of a 6ms frame budget with 144hz
monitors. Is this wise?
 

[fdoray, 2016/12/01 22:30:43]

Yes they are. The whole implementation file is guarded by a DCHECK_IS_ON()
https://cs.chromium.org/chromium/src/base/threading/thread_restrictions.cc?rc....
There is a no-op implementation when !DCHECK_IS_ON()
https://cs.chromium.org/chromium/src/base/threading/thread_restrictions.h?sq=...

 
     {
       // Set up SequenceToken as expected for the scope of the task.
       ScopedSetSequenceTokenForCurrentThread
           scoped_set_sequence_token_for_current_thread(sequence_token);
 
       // Set up TaskRunnerHandle as expected for the scope of the task.
       std::unique_ptr<SequencedTaskRunnerHandle> sequenced_task_runner_handle;
       std::unique_ptr<ThreadTaskRunnerHandle> single_thread_task_runner_handle;
       DCHECK(!task->sequenced_task_runner_ref ||
@@ skipping 16 matching lines @@
       // TODO(gab): In a better world this would be tacked on as an extra arg
       // to the trace event generated above. This is not possible however until
       // http://crbug.com/652692 is resolved.
       TRACE_EVENT1("task_scheduler", "TaskTracker::RunTask", "task_info",
                    MakeUnique<TaskTracingInfo>(task->traits, execution_mode,
                                                sequence_token));
 
       PerformRunTask(std::move(task));
     }
 
+    ThreadRestrictions::SetIOAllowed(previous_io_allowed);
+    ThreadRestrictions::SetSingletonAllowed(previous_singleton_allowed);
+
     AfterRunTask(shutdown_behavior);
   }
 
   if (!is_delayed)
     DecrementNumPendingUndelayedTasks();
 
   return can_run_task;
 }
 
 bool TaskTracker::HasShutdownStarted() const {
@@ skipping 168 matching lines @@
       subtle::NoBarrier_AtomicIncrement(&num_pending_undelayed_tasks_, -1);
   DCHECK_GE(new_num_pending_undelayed_tasks, 0);
   if (new_num_pending_undelayed_tasks == 0) {
     AutoSchedulerLock auto_lock(flush_lock_);
     flush_cv_->Signal();
   }
 }
 
 }  // namespace internal
 }  // namespace base