Update task tracking to not depend on message_loop_ singleton

base/tracked_objects.cc

Index: base/tracked_objects.cc
===================================================================
--- base/tracked_objects.cc	(revision 104925)
+++ base/tracked_objects.cc	(working copy)
@@ -24,55 +24,45 @@
 // static
 AutoTracking::State AutoTracking::state_ = AutoTracking::kNeverBeenRun;
 
+// A locked protected counter to assign sequence number to threads.
+// static
+int ThreadData::thread_number_counter = 0;
+
 //------------------------------------------------------------------------------
 // Death data tallies durations when a death takes place.
 
 void DeathData::RecordDeath(const TimeDelta& duration) {
   ++count_;
   life_duration_ += duration;
-  int64 milliseconds = duration.InMilliseconds();
-  square_duration_ += milliseconds * milliseconds;
 }
 
 int DeathData::AverageMsDuration() const {
   return static_cast<int>(life_duration_.InMilliseconds() / count_);
 }
 
-double DeathData::StandardDeviation() const {
-  double average = AverageMsDuration();
-  double variance = static_cast<float>(square_duration_)/count_
-                    - average * average;
-  return sqrt(variance);
-}
-
-
 void DeathData::AddDeathData(const DeathData& other) {
   count_ += other.count_;
   life_duration_ += other.life_duration_;
-  square_duration_ += other.square_duration_;
 }
 
 void DeathData::Write(std::string* output) const {
   if (!count_)
     return;
-  if (1 == count_) {
-    base::StringAppendF(output, "(1)Life in %dms ", AverageMsDuration());
-  } else {
-    base::StringAppendF(output, "(%d)Lives %dms/life ",
-                        count_, AverageMsDuration());
-  }
+  base::StringAppendF(output, "Lives:%d, Total:%dms Avg:%dms/life ",
+                      count_,
+                      static_cast<int>(life_duration_.InMilliseconds()),

[awong, 2011/10/14 23:29:02]

Would it be worth using the 64-bit compatibility format macros and avoid the
cast?

http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml#64-bit_Portabi...

[jar (doing other things), 2011/10/15 15:11:33]

Done.

I'll be surprised when we have more than 24days of milliseconds.... but I guess
this way I don't need to worry about it.


On 2011/10/14 23:29:02, awong wrote:

+                      AverageMsDuration());
 }
 
 void DeathData::Clear() {
   count_ = 0;
   life_duration_ = TimeDelta();
-  square_duration_ = 0;
 }
 
 //------------------------------------------------------------------------------
 BirthOnThread::BirthOnThread(const Location& location)
     : location_(location),
-      birth_thread_(ThreadData::current()) { }
+      birth_thread_(ThreadData::FactoryGet(NULL)) { }
 
 //------------------------------------------------------------------------------
 Births::Births(const Location& location)
@@ -90,45 +80,61 @@
 // static
 ThreadData::Status ThreadData::status_ = ThreadData::UNINITIALIZED;
 
-ThreadData::ThreadData() : next_(NULL) {
-  // This shouldn't use the MessageLoop::current() LazyInstance since this might
-  // be used on a non-joinable thread.
-  // http://crbug.com/62728
-  base::ThreadRestrictions::ScopedAllowSingleton scoped_allow_singleton;
-  message_loop_ = MessageLoop::current();
+ThreadData::ThreadData(const char* suggested_name)
+    : next_(NULL),
+      thread_number_(0) {
+  if (suggested_name) {
+    DCHECK_GE(strlen(suggested_name), 0u);
+    thread_name_ = suggested_name;
+    return;
+  }
+  {
+    base::AutoLock lock(list_lock_);
+    thread_number_ = ++thread_number_counter;
+  }
+  base::StringAppendF(&thread_name_, "WorkerThread-%d", thread_number_);
 }
 
 ThreadData::~ThreadData() {}
 
 // static
-ThreadData* ThreadData::current() {
+ThreadData* ThreadData::FactoryGet(const char* suggested_name) {
   if (!tls_index_.initialized())
     return NULL;
 
-  ThreadData* registry = static_cast<ThreadData*>(tls_index_.Get());
-  if (!registry) {
-    // We have to create a new registry for ThreadData.
+  ThreadData* registered = static_cast<ThreadData*>(tls_index_.Get());
+  if (!registered) {
+    // We have to create a new registry entry for this ThreadData.
     bool too_late_to_create = false;
+    // TODO(jar): Host all unamed (Worker) threads in *one* ThreadData instance,
+    // (with locking protection on that instance) or else recycle and re-use
+    // worker thread ThreadData when the worker thread terminates.
+    registered = new ThreadData(suggested_name);
     {
-      registry = new ThreadData;
       base::AutoLock lock(list_lock_);
       // Use lock to insure we have most recent status.
       if (!IsActive()) {
         too_late_to_create = true;
       } else {
-        // Use lock to insert into list.
-        registry->next_ = first_;
-        first_ = registry;
+        // Use list_lock_ to insert as new head of list.
+        registered->next_ = first_;
+        first_ = registered;
       }
     }  // Release lock.
     if (too_late_to_create) {
-      delete registry;
-      registry = NULL;
+      delete registered;
+      registered = NULL;
     } else {
-      tls_index_.Set(registry);
+      tls_index_.Set(registered);
     }
   }
-  return registry;
+  DCHECK_GT(registered->thread_name_.size(), 0u);
+  // If this DCHECK fails, then a task was *probabably* created on this thread
+  // before the thread object was initialized with a name etc.
+  DCHECK(!suggested_name ||
+         registered->thread_name_.compare(suggested_name) == 0);
+
+  return registered;
 }
 
 // static
@@ -136,7 +142,6 @@
   if (!ThreadData::IsActive())
     return;  // Not yet initialized.
 
-  DCHECK(ThreadData::current());
   DataCollector collected_data;  // Gather data.
   collected_data.AddListOfLivingObjects();  // Add births that are still alive.
 
@@ -166,7 +171,8 @@
   const char* help_string = "The following are the keywords that can be used to"
     " sort and aggregate the data, or to select data.<br><ul>"
     "<li><b>count</b> Number of instances seen."
-    "<li><b>duration</b> Duration in ms from construction to descrution."
+    "<li><b>duration</b> Average duration in ms of Run() time."
+    "<li><b>totalduration</b> Summed durations in ms of Run() times."
     "<li><b>birth</b> Thread on which the task was constructed."
     "<li><b>death</b> Thread on which the task was run and deleted."
     "<li><b>file</b> File in which the task was contructed."
@@ -234,15 +240,6 @@
 }
 
 Births* ThreadData::TallyABirth(const Location& location) {
-  {
-    // This shouldn't use the MessageLoop::current() LazyInstance since this
-    // might be used on a non-joinable thread.
-    // http://crbug.com/62728
-    base::ThreadRestrictions::ScopedAllowSingleton scoped_allow_singleton;
-    if (!message_loop_)  // In case message loop wasn't yet around...
-      message_loop_ = MessageLoop::current();  // Find it now.
-  }
-
   BirthMap::iterator it = birth_map_.find(location);
   if (it != birth_map_.end()) {
     it->second->RecordBirth();
@@ -257,43 +254,50 @@
   return tracker;
 }
 
-void ThreadData::TallyADeath(const Births& lifetimes,
+void ThreadData::TallyADeath(const Births& the_birth,
                              const TimeDelta& duration) {
-  {
-    // http://crbug.com/62728
-    base::ThreadRestrictions::ScopedAllowSingleton scoped_allow_singleton;
-    if (!message_loop_)  // In case message loop wasn't yet around...
-      message_loop_ = MessageLoop::current();  // Find it now.
-  }
-
-  DeathMap::iterator it = death_map_.find(&lifetimes);
+  DeathMap::iterator it = death_map_.find(&the_birth);
   if (it != death_map_.end()) {
     it->second.RecordDeath(duration);
     return;
   }
 
   base::AutoLock lock(lock_);  // Lock since the map may get relocated now.
-  death_map_[&lifetimes].RecordDeath(duration);
+  death_map_[&the_birth].RecordDeath(duration);
 }
 
 // static
 Births* ThreadData::TallyABirthIfActive(const Location& location) {
-  if (IsActive()) {
-    ThreadData* current_thread_data = current();
-    if (current_thread_data) {
-      return current_thread_data->TallyABirth(location);
-    }
-  }
-
-  return NULL;
+#if !defined(TRACK_ALL_TASK_OBJECTS)
+  return NULL;  // Not compiled in.
+#endif

[awong, 2011/10/14 23:29:02]

It'd be nice to just #else the rest of the function rather than relying on
compiler deadbranch elimination.

[jar (doing other things), 2011/10/15 15:11:33]

Done.

+  if (!IsActive())
+    return NULL;
+  ThreadData* current_thread_data = FactoryGet(NULL);
+  if (!current_thread_data)
+    return NULL;
+  return current_thread_data->TallyABirth(location);
 }
 
 // static
 void ThreadData::TallyADeathIfActive(const Births* the_birth,
-                                     const base::TimeDelta& duration) {
-  if (IsActive() && the_birth) {
-    current()->TallyADeath(*the_birth, duration);
-  }
+                                     const base::TimeTicks& time_posted,
+                                     const base::TimeTicks& start_of_run,
+                                     const MessageLoop* message_loop) {
+#if !defined(TRACK_ALL_TASK_OBJECTS)
+  return;  // Not compiled in.
+#endif

[awong, 2011/10/14 23:29:02]

Same as above.

[jar (doing other things), 2011/10/15 15:11:33]

Done.

+  if (!IsActive() || !the_birth)
+    return;
+  ThreadData* current_thread_data = FactoryGet(NULL);
+  if (!current_thread_data)
+    return;
+  DCHECK(!message_loop ||
+         message_loop->thread_name() == current_thread_data->thread_name_);
+  // We don't currently handle queueing time duration, since time_posted, so
+  // we discard time_posted.

[awong, 2011/10/14 23:29:02]

Should this be a TODO with a bug?

[jar (doing other things), 2011/10/15 15:11:33]

I went ahead and implemented queue duration tallying as well.

+  current_thread_data->TallyADeath(*the_birth,
+                                   base::TimeTicks::Now() - start_of_run);
 }
 
 // static
@@ -302,12 +306,6 @@
   return first_;
 }
 
-const std::string ThreadData::ThreadName() const {
-  if (message_loop_)
-    return message_loop_->thread_name();
-  return "ThreadWithoutMessageLoop";
-}
-
 // This may be called from another thread.
 void ThreadData::SnapshotBirthMap(BirthMap *output) const {
   base::AutoLock lock(lock_);
@@ -326,7 +324,7 @@
 
 // static
 void ThreadData::ResetAllThreadData() {
-  ThreadData* my_list = ThreadData::current()->first();
+  ThreadData* my_list = ThreadData::FactoryGet(NULL)->first();
 
   for (ThreadData* thread_data = my_list;
        thread_data;
@@ -344,103 +342,6 @@
     it->second->Clear();
 }
 
-#ifdef OS_WIN
-// A class used to count down which is accessed by several threads.  This is
-// used to make sure RunOnAllThreads() actually runs a task on the expected
-// count of threads.
-class ThreadData::ThreadSafeDownCounter {
- public:
-  // Constructor sets the count, once and for all.
-  explicit ThreadSafeDownCounter(size_t count);
-
-  // Decrement the count, and return true if we hit zero.  Also delete this
-  // instance automatically when we hit zero.
-  bool LastCaller();
-
- private:
-  size_t remaining_count_;
-  base::Lock lock_;  // protect access to remaining_count_.
-};
-
-ThreadData::ThreadSafeDownCounter::ThreadSafeDownCounter(size_t count)
-    : remaining_count_(count) {
-  DCHECK_GT(remaining_count_, 0u);
-}
-
-bool ThreadData::ThreadSafeDownCounter::LastCaller() {
-  {
-    base::AutoLock lock(lock_);
-    if (--remaining_count_)
-      return false;
-  }  // Release lock, so we can delete everything in this instance.
-  delete this;
-  return true;
-}
-
-// A Task class that runs a static method supplied, and checks to see if this
-// is the last tasks instance (on last thread) that will run the method.
-// IF this is the last run, then the supplied event is signalled.
-class ThreadData::RunTheStatic : public Task {
- public:
-  typedef void (*FunctionPointer)();
-  RunTheStatic(FunctionPointer function,
-               HANDLE completion_handle,
-               ThreadSafeDownCounter* counter);
-  // Run the supplied static method, and optionally set the event.
-  void Run();
-
- private:
-  FunctionPointer function_;
-  HANDLE completion_handle_;
-  // Make sure enough tasks are called before completion is signaled.
-  ThreadSafeDownCounter* counter_;
-
-  DISALLOW_COPY_AND_ASSIGN(RunTheStatic);
-};
-
-ThreadData::RunTheStatic::RunTheStatic(FunctionPointer function,
-                                       HANDLE completion_handle,
-                                       ThreadSafeDownCounter* counter)
-    : function_(function),
-      completion_handle_(completion_handle),
-      counter_(counter) {
-}
-
-void ThreadData::RunTheStatic::Run() {
-  function_();
-  if (counter_->LastCaller())
-    SetEvent(completion_handle_);
-}
-
-// TODO(jar): This should use condition variables, and be cross platform.
-void ThreadData::RunOnAllThreads(void (*function)()) {
-  ThreadData* list = first();  // Get existing list.
-
-  std::vector<MessageLoop*> message_loops;
-  for (ThreadData* it = list; it; it = it->next()) {
-    if (current() != it && it->message_loop())
-      message_loops.push_back(it->message_loop());
-  }
-
-  ThreadSafeDownCounter* counter =
-    new ThreadSafeDownCounter(message_loops.size() + 1);  // Extra one for us!
-
-  HANDLE completion_handle = CreateEvent(NULL, false, false, NULL);
-  // Tell all other threads to run.
-  for (size_t i = 0; i < message_loops.size(); ++i)
-    message_loops[i]->PostTask(
-        FROM_HERE, new RunTheStatic(function, completion_handle, counter));
-
-  // Also run Task on our thread.
-  RunTheStatic local_task(function, completion_handle, counter);
-  local_task.Run();
-
-  WaitForSingleObject(completion_handle, INFINITE);
-  int ret_val = CloseHandle(completion_handle);
-  DCHECK(ret_val);
-}
-#endif  // OS_WIN
-
 // static
 bool ThreadData::StartTracking(bool status) {
 #if !defined(TRACK_ALL_TASK_OBJECTS)
@@ -465,24 +366,14 @@
   return status_ == ACTIVE;
 }
 
-#ifdef OS_WIN
 // static
-void ThreadData::ShutdownMultiThreadTracking() {
-  // Using lock, guarantee that no new ThreadData instances will be created.
-  if (!StartTracking(false))
-    return;
-
-  RunOnAllThreads(ShutdownDisablingFurtherTracking);
-
-  // Now the *only* threads that might change the database are the threads with
-  // no messages loops.  They might still be adding data to their birth records,
-  // but since no objects are deleted on those threads, there will be no further
-  // access to to cross-thread data.
-  // We could do a cleanup on all threads except for the ones without
-  // MessageLoops, but we won't bother doing cleanup (destruction of data) yet.
-  return;
-}
+base::TimeTicks ThreadData::Now() {
+#if defined(TRACK_ALL_TASK_OBJECTS)
+  if (status_ == ACTIVE)
+    return base::TimeTicks::Now();
 #endif
+  return base::TimeTicks();  // Super fast when disabled, or not compiled in.
+}
 
 // static
 void ThreadData::ShutdownSingleThreadedCleanup() {
@@ -541,15 +432,15 @@
 
 const std::string Snapshot::DeathThreadName() const {
   if (death_thread_)
-    return death_thread_->ThreadName();
+    return death_thread_->thread_name();
   return "Still_Alive";
 }
 
 void Snapshot::Write(std::string* output) const {
   death_data_.Write(output);
   base::StringAppendF(output, "%s->%s ",
-                      birth_->birth_thread()->ThreadName().c_str(),
-                      death_thread_->ThreadName().c_str());
+                      birth_->birth_thread()->thread_name().c_str(),
+                      death_thread_->thread_name().c_str());
   birth_->location().Write(true, true, output);
 }
 
@@ -564,7 +455,7 @@
   DCHECK(ThreadData::IsActive());
 
   // Get an unchanging copy of a ThreadData list.
-  ThreadData* my_list = ThreadData::current()->first();
+  ThreadData* my_list = ThreadData::FactoryGet(NULL)->first();
 
   count_of_contributing_threads_ = 0;
   for (ThreadData* thread_data = my_list;
@@ -573,16 +464,12 @@
     ++count_of_contributing_threads_;
   }
 
-  // Gather data serially.  A different constructor could be used to do in
-  // parallel, and then invoke an OnCompletion task.
+  // Gather data serially.
   // This hackish approach *can* get some slighly corrupt tallies, as we are
   // grabbing values without the protection of a lock, but it has the advantage
   // of working even with threads that don't have message loops.  If a user
   // sees any strangeness, they can always just run their stats gathering a
   // second time.
-  // TODO(jar): Provide version that gathers stats safely via PostTask in all
-  // cases where thread_data supplies a message_loop to post to.  Be careful to
-  // handle message_loops that are destroyed!?!
   for (ThreadData* thread_data = my_list;
        thread_data;
        thread_data = thread_data->next()) {
@@ -681,7 +568,7 @@
                         birth_threads_.size());
   } else {
     base::StringAppendF(output, "All born on %s. ",
-                        birth_threads_.begin()->first->ThreadName().c_str());
+                        birth_threads_.begin()->first->thread_name().c_str());
   }
 
   if (death_threads_.size() > 1) {
@@ -690,7 +577,7 @@
   } else {
     if (death_threads_.begin()->first) {
       base::StringAppendF(output, "All deleted on %s. ",
-                          death_threads_.begin()->first->ThreadName().c_str());
+                          death_threads_.begin()->first->thread_name().c_str());
     } else {
       output->append("All these objects are still alive.");
     }
@@ -735,10 +622,10 @@
   switch (selector_) {
     case BIRTH_THREAD:
       if (left.birth_thread() != right.birth_thread() &&
-          left.birth_thread()->ThreadName() !=
-          right.birth_thread()->ThreadName())
-        return left.birth_thread()->ThreadName() <
-            right.birth_thread()->ThreadName();
+          left.birth_thread()->thread_name() !=
+          right.birth_thread()->thread_name())
+        return left.birth_thread()->thread_name() <
+            right.birth_thread()->thread_name();
       break;
 
     case DEATH_THREAD:
@@ -790,6 +677,13 @@
         return left.AverageMsDuration() > right.AverageMsDuration();
       break;
 
+    case TOTAL_DURATION:
+      if (!left.count() || !right.count())
+        break;
+      if (left.life_duration() != right.life_duration())
+        return left.life_duration() > right.life_duration();
+      break;
+
     default:
       break;
   }
@@ -807,8 +701,8 @@
   switch (selector_) {
     case BIRTH_THREAD:
       if (left.birth_thread() != right.birth_thread() &&
-          left.birth_thread()->ThreadName() !=
-              right.birth_thread()->ThreadName())
+          left.birth_thread()->thread_name() !=
+              right.birth_thread()->thread_name())
         return false;
       break;
 
@@ -837,13 +731,9 @@
       break;
 
     case COUNT:
-      if (left.count() != right.count())
-        return false;
-      break;
-
     case AVERAGE_DURATION:
-      if (left.life_duration() != right.life_duration())
-        return false;
+    case TOTAL_DURATION:
+      // We don't produce separate aggretation when only counts or times differ.
       break;
 
     default:
@@ -858,7 +748,7 @@
   if (required_.size()) {
     switch (selector_) {
       case BIRTH_THREAD:
-        if (sample.birth_thread()->ThreadName().find(required_) ==
+        if (sample.birth_thread()->thread_name().find(required_) ==
             std::string::npos)
           return false;
         break;
@@ -934,13 +824,14 @@
     initialized = true;
     // Sorting and aggretation keywords, which specify how to sort the data, or
     // can specify a required match from the specified field in the record.
-    key_map["count"]    = COUNT;
-    key_map["duration"] = AVERAGE_DURATION;
-    key_map["birth"]    = BIRTH_THREAD;
-    key_map["death"]    = DEATH_THREAD;
-    key_map["file"]     = BIRTH_FILE;
-    key_map["function"] = BIRTH_FUNCTION;
-    key_map["line"]     = BIRTH_LINE;
+    key_map["count"]         = COUNT;
+    key_map["totalduration"] = TOTAL_DURATION;
+    key_map["duration"]      = AVERAGE_DURATION;
+    key_map["birth"]         = BIRTH_THREAD;
+    key_map["death"]         = DEATH_THREAD;
+    key_map["file"]          = BIRTH_FILE;
+    key_map["function"]      = BIRTH_FUNCTION;
+    key_map["line"]          = BIRTH_LINE;
 
     // Immediate commands that do not involve setting sort order.
     key_map["reset"]     = RESET_ALL_DATA;
@@ -976,6 +867,7 @@
 
   // Select subgroup ordering (if we want to display the subgroup)
   SetSubgroupTiebreaker(COUNT);
+  SetSubgroupTiebreaker(TOTAL_DURATION);
   SetSubgroupTiebreaker(AVERAGE_DURATION);
   SetSubgroupTiebreaker(BIRTH_THREAD);
   SetSubgroupTiebreaker(DEATH_THREAD);
@@ -992,7 +884,7 @@
   switch (selector_) {
     case BIRTH_THREAD:
       base::StringAppendF(output, "All new on %s ",
-                          sample.birth_thread()->ThreadName().c_str());
+                          sample.birth_thread()->thread_name().c_str());
       wrote_data = true;
       break;
 
@@ -1033,7 +925,7 @@
       !(combined_selectors_ & DEATH_THREAD))
     base::StringAppendF(output, "%s->%s ",
                         (combined_selectors_ & BIRTH_THREAD) ? "*" :
-                          sample.birth().birth_thread()->ThreadName().c_str(),
+                          sample.birth().birth_thread()->thread_name().c_str(),
                         (combined_selectors_ & DEATH_THREAD) ? "*" :
                           sample.DeathThreadName().c_str());
   sample.birth().location().Write(!(combined_selectors_ & BIRTH_FILE),