Lazily initialize MessageLoop for faster thread startup

base/threading/thread.cc

 // Copyright (c) 2012 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/threading/thread.h"
 
 #include "base/bind.h"
 #include "base/lazy_instance.h"
 #include "base/profiler/scoped_tracker.h"
 #include "base/synchronization/waitable_event.h"
+#include "base/synchronization/waitable_event_watcher.h"
 #include "base/third_party/dynamic_annotations/dynamic_annotations.h"
 #include "base/threading/thread_id_name_manager.h"
 #include "base/threading/thread_local.h"
 #include "base/threading/thread_restrictions.h"
 
 #if defined(OS_WIN)
 #include "base/win/scoped_com_initializer.h"
 #endif
 
 namespace base {
 
 namespace {
 
 // We use this thread-local variable to record whether or not a thread exited
 // because its Stop method was called.  This allows us to catch cases where
 // MessageLoop::QuitWhenIdle() is called directly, which is unexpected when
 // using a Thread to setup and run a MessageLoop.
 base::LazyInstance<base::ThreadLocalBoolean> lazy_tls_bool =
     LAZY_INSTANCE_INITIALIZER;
 
 }  // namespace
 
 // This is used to trigger the message loop to exit.
 void ThreadQuitHelper() {
   MessageLoop::current()->QuitWhenIdle();
   Thread::SetThreadWasQuitProperly(true);
 }
 
-// Used to pass data to ThreadMain.  This structure is allocated on the stack
-// from within StartWithOptions.
-struct Thread::StartupData {
-  // We get away with a const reference here because of how we are allocated.
-  const Thread::Options& options;
-
-  // Used to synchronize thread startup.
-  WaitableEvent event;
-
-  explicit StartupData(const Options& opt)
-      : options(opt),
-        event(false, false) {}
-};
-
 Thread::Options::Options()
     : message_loop_type(MessageLoop::TYPE_DEFAULT),
       timer_slack(TIMER_SLACK_NONE),
       stack_size(0) {
 }
 
 Thread::Options::Options(MessageLoop::Type type,
                          size_t size)
     : message_loop_type(type),
       timer_slack(TIMER_SLACK_NONE),
       stack_size(size) {
 }
 
 Thread::Options::~Options() {
 }
 
 Thread::Thread(const std::string& name)
     :
 #if defined(OS_WIN)
       com_status_(NONE),
 #endif
       started_(false),
       stopping_(false),
       running_(false),
-      startup_data_(NULL),
       thread_(0),
-      message_loop_(NULL),
+      message_loop_(nullptr),
       thread_id_(kInvalidThreadId),
       name_(name) {
 }
 
 Thread::~Thread() {
   Stop();
 }
 
 bool Thread::Start() {
   Options options;
@@ skipping 11 matching lines @@
         "465458 base::Thread::StartWithOptions"));
 
   DCHECK(!message_loop_);
 #if defined(OS_WIN)
   DCHECK((com_status_ != STA) ||
       (options.message_loop_type == MessageLoop::TYPE_UI));
 #endif
 
   SetThreadWasQuitProperly(false);
 
-  StartupData startup_data(options);
-  startup_data_ = &startup_data;
+  scoped_ptr<MessageLoop::LazyInitOptions> message_loop_options(

[DaleCurtis, 2015/03/18 22:04:28]

No need for a scoped_ptr allocation? Seems simple enough to allow pass-by-copy.

[kinuko, 2015/03/19 03:08:16]

Not really, but I want to keep around this object in MessageLoop class only
until message loop is initialized on the target thread, so allocating this here
and just passing around the scoped_ptr seems most efficient. (Are you worrying
about slight jank by allocation?)

+      new MessageLoop::LazyInitOptions);
+  message_loop_options->message_loop_type = options.message_loop_type;
+  message_loop_options->message_pump_factory = options.message_pump_factory;
+  message_loop_options->timer_slack = options.timer_slack;
+  message_loop_ = MessageLoop::CreateForLazyInit(message_loop_options.Pass());
+
+  start_event_.reset(new WaitableEvent(false, false));
 
   if (!PlatformThread::Create(options.stack_size, this, &thread_)) {
     DLOG(ERROR) << "failed to create thread";
-    startup_data_ = NULL;
+    delete message_loop_;
+    message_loop_ = NULL;
     return false;
   }
 
-  // TODO(eroman): Remove once crbug.com/465458 is solved.
-  tracked_objects::ScopedTracker tracking_profile_wait(
-      FROM_HERE_WITH_EXPLICIT_FUNCTION(
-          "465458 base::Thread::StartWithOptions (Wait)"));
-
-  // Wait for the thread to start and initialize message_loop_
-  base::ThreadRestrictions::ScopedAllowWait allow_wait;
-  startup_data.event.Wait();
-
-  // set it to NULL so we don't keep a pointer to some object on the stack.
-  startup_data_ = NULL;
   started_ = true;
 
   DCHECK(message_loop_);
   return true;
 }
 
+bool Thread::StartAndWait() {
+  bool result = Start();
+  if (!result)
+    return result;
+  WaitUntilThreadStarted();
+  return result;
+}
+
+bool Thread::WaitUntilThreadStarted() {
+  if (!started_)
+    return false;
+  base::ThreadRestrictions::ScopedAllowWait allow_wait;
+  start_event_->Wait();
+  return true;
+}
+
 void Thread::Stop() {
   if (!started_)
     return;
 
   StopSoon();
 
   // Wait for the thread to exit.
   //
   // TODO(darin): Unfortunately, we need to keep message_loop_ around until
   // the thread exits.  Some consumers are abusing the API.  Make them stop.
@@ skipping 17 matching lines @@
   DCHECK_NE(ANNOTATE_UNPROTECTED_READ(thread_id_), PlatformThread::CurrentId());
 
   if (stopping_ || !message_loop_)
     return;
 
   stopping_ = true;
   message_loop_->PostTask(FROM_HERE, base::Bind(&ThreadQuitHelper));
 }
 
 bool Thread::IsRunning() const {
-  return running_;
+  if (stopping_)
+    return IsInRunLoop();
+  return started_;
 }
 
 void Thread::SetPriority(ThreadPriority priority) {
   // The thread must be started (and id known) for this to be
   // compatible with all platforms.
   DCHECK_NE(thread_id_, kInvalidThreadId);
   PlatformThread::SetThreadPriority(thread_, priority);
 }
 
 void Thread::Run(MessageLoop* message_loop) {
   message_loop->Run();
 }
 
 void Thread::SetThreadWasQuitProperly(bool flag) {
   lazy_tls_bool.Pointer()->Set(flag);
 }
 
 bool Thread::GetThreadWasQuitProperly() {
   bool quit_properly = true;
 #ifndef NDEBUG
   quit_properly = lazy_tls_bool.Pointer()->Get();
 #endif
   return quit_properly;
 }
 
 void Thread::ThreadMain() {
   {
-    // The message loop for this thread.
-    // Allocated on the heap to centralize any leak reports at this line.
-    scoped_ptr<MessageLoop> message_loop;
-    if (!startup_data_->options.message_pump_factory.is_null()) {
-      message_loop.reset(
-          new MessageLoop(startup_data_->options.message_pump_factory.Run()));
-    } else {
-      message_loop.reset(
-          new MessageLoop(startup_data_->options.message_loop_type));
-    }
-
     // Complete the initialization of our Thread object.
     thread_id_ = PlatformThread::CurrentId();
     PlatformThread::SetName(name_.c_str());
     ANNOTATE_THREAD_NAME(name_.c_str());  // Tell the name to race detector.
-    message_loop->set_thread_name(name_);
-    message_loop->SetTimerSlack(startup_data_->options.timer_slack);
-    message_loop_ = message_loop.get();
+
+    // Lazily initialize the message_loop so that it can run on this thread.
+    DCHECK(message_loop_);
+    scoped_ptr<MessageLoop> message_loop(message_loop_);
+    message_loop_->set_thread_name(name_);
+    message_loop_->LazyInit();
 
 #if defined(OS_WIN)
     scoped_ptr<win::ScopedCOMInitializer> com_initializer;
     if (com_status_ != NONE) {
       com_initializer.reset((com_status_ == STA) ?
           new win::ScopedCOMInitializer() :
           new win::ScopedCOMInitializer(win::ScopedCOMInitializer::kMTA));
     }
 #endif
 
     // Let the thread do extra initialization.
-    // Let's do this before signaling we are started.
     Init();
 
-    running_ = true;
-    startup_data_->event.Signal();
-    // startup_data_ can't be touched anymore since the starting thread is now
-    // unlocked.
+    {

[DaleCurtis, 2015/03/18 22:04:28]

You can combine running_ and start_event_ into a running_event_ and check the
signaled state.

[kinuko, 2015/03/19 03:08:16]

Sounds like a good idea, done.

[kinuko, 2015/03/20 08:44:22]

Hmm, replacing running_ with event_->IsSignaled() somehow breaks an Android
test, let me revert this change for now. (Instead I combined started_ into
start_event_)

+      base::AutoLock lock(running_lock_);
+      running_ = true;
+    }
+
+    start_event_->Signal();
 
     Run(message_loop_);
-    running_ = false;
+
+    {
+      base::AutoLock lock(running_lock_);
+      running_ = false;
+    }
 
     // Let the thread do extra cleanup.
     CleanUp();
 
 #if defined(OS_WIN)
     com_initializer.reset();
 #endif
 
     // Assert that MessageLoop::Quit was called by ThreadQuitHelper.
     DCHECK(GetThreadWasQuitProperly());
 
     // We can't receive messages anymore.
     message_loop_ = NULL;
   }
 }
 
+bool Thread::IsInRunLoop() const {
+  base::AutoLock lock(running_lock_);
+  return running_;
+}
+
 }  // namespace base