Update task tracking to not depend on message_loop_ singleton

base/tracked_objects.cc

 // Copyright (c) 2011 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/tracked_objects.h"
 
 #include <math.h>
 
 #include "base/format_macros.h"
 #include "base/message_loop.h"
 #include "base/string_util.h"
 #include "base/stringprintf.h"
 #include "base/threading/thread_restrictions.h"
 
 using base::TimeDelta;
 
 namespace tracked_objects {
 
 // A TLS slot to the TrackRegistry for the current thread.
 // static
 base::ThreadLocalStorage::Slot ThreadData::tls_index_(base::LINKER_INITIALIZED);
 
 // A global state variable to prevent repeated initialization during tests.
 // 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()),
+                      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)
     : BirthOnThread(location),
       birth_count_(1) { }
 
 //------------------------------------------------------------------------------
 // ThreadData maintains the central data for all births and death.
 
 // static
 ThreadData* ThreadData::first_ = NULL;
 // static
 base::Lock ThreadData::list_lock_;
 
 // 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
 void ThreadData::WriteHTML(const std::string& query, std::string* output) {
   if (!ThreadData::IsActive())
     return;  // Not yet initialized.
 
-  DCHECK(ThreadData::current());
   DataCollector collected_data;  // Gather data.
   collected_data.AddListOfLivingObjects();  // Add births that are still alive.
 
   // Data Gathering is complete. Now to sort/process/render.
   DataCollector::Collection* collection = collected_data.collection();
 
   // Create filtering and sort comparison object.
   Comparator comparator;
   comparator.ParseQuery(query);
 
   // Filter out acceptable (matching) instances.
   DataCollector::Collection match_array;
   for (DataCollector::Collection::iterator it = collection->begin();
        it != collection->end(); ++it) {
     if (comparator.Acceptable(*it))
       match_array.push_back(*it);
   }
 
   comparator.Sort(&match_array);
 
   WriteHTMLTotalAndSubtotals(match_array, comparator, output);
 
   comparator.Clear();  // Delete tiebreaker_ instances.
 
   output->append("</pre>");
 
   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."
     "<li><b>function</b> Function in which the task was constructed."
     "<li><b>line</b> Line number of the file in which the task was constructed."
     "</ul><br>"
     "As examples:<ul>"
     "<li><b>about:tracking/file</b> would sort the above data by file, and"
     " aggregate data on a per-file basis."
     "<li><b>about:tracking/file=Dns</b> would only list data for tasks"
@@ skipping 47 matching lines @@
         output->append("<br>");
         subtotals.Write(output);
         output->append("<br><hr><br>");
         subtotals.Clear();
       }
     }
   }
 }
 
 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

[jar (doing other things), 2011/10/14 02:29:53]

This was the primary motivation for this cleanup CL.

-    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();
     return it->second;
   }
 
   Births* tracker = new Births(location);
   // Lock since the map may get relocated now, and other threads sometimes
   // snapshot it (but they lock before copying it).
   base::AutoLock lock(lock_);
   birth_map_[location] = tracker;
   return tracker;
 }
 
-void ThreadData::TallyADeath(const Births& lifetimes,
+void ThreadData::TallyADeath(const Births& the_birth,
                              const TimeDelta& duration) {
-  {
-    // http://crbug.com/62728

[jar (doing other things), 2011/10/14 02:29:53]

Again... the motivation for the CL.

-    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
+  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
+  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.
+  current_thread_data->TallyADeath(*the_birth,
+                                   base::TimeTicks::Now() - start_of_run);
 }
 
 // static
 ThreadData* ThreadData::first() {
   base::AutoLock lock(list_lock_);
   return first_;
 }
 
-const std::string ThreadData::ThreadName() const {
-  if (message_loop_)
-    return message_loop_->thread_name();

[jar (doing other things), 2011/10/14 02:29:53]

We now store the thread name, and don't need to keep looking it up.

-  return "ThreadWithoutMessageLoop";
-}
-
 // This may be called from another thread.
 void ThreadData::SnapshotBirthMap(BirthMap *output) const {
   base::AutoLock lock(lock_);
   for (BirthMap::const_iterator it = birth_map_.begin();
        it != birth_map_.end(); ++it)
     (*output)[it->first] = it->second;
 }
 
 // This may be called from another thread.
 void ThreadData::SnapshotDeathMap(DeathMap *output) const {
   base::AutoLock lock(lock_);
   for (DeathMap::const_iterator it = death_map_.begin();
        it != death_map_.end(); ++it)
     (*output)[it->first] = it->second;
 }
 
 // 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;
        thread_data = thread_data->next())
     thread_data->Reset();
 }
 
 void ThreadData::Reset() {
   base::AutoLock lock(lock_);
   for (DeathMap::iterator it = death_map_.begin();
        it != death_map_.end(); ++it)
     it->second.Clear();
   for (BirthMap::iterator it = birth_map_.begin();
        it != birth_map_.end(); ++it)
     it->second->Clear();
 }
 
-#ifdef OS_WIN
-// A class used to count down which is accessed by several threads.  This is

[jar (doing other things), 2011/10/14 02:29:53]

We no longer do any terminaition cleanup... so all this code goes away.

-// 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());

[jar (doing other things), 2011/10/14 02:29:53]

Since we don't do this racy cleanup attempt, we don't need these message
loops... and they are gone, as well as this code.

-  }
-
-  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)
   return false;  // Not compiled in.
 #endif
 
   if (!status) {
     base::AutoLock lock(list_lock_);
     DCHECK(status_ == ACTIVE || status_ == SHUTDOWN);
     status_ = SHUTDOWN;
     return true;
   }
   base::AutoLock lock(list_lock_);
   DCHECK_EQ(UNINITIALIZED, status_);
   CHECK(tls_index_.Initialize(NULL));
   status_ = ACTIVE;
   return true;
 }
 
 // static
 bool ThreadData::IsActive() {
   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.
 }
-#endif
 
 // static
 void ThreadData::ShutdownSingleThreadedCleanup() {
+  NOTREACHED();

[jar (doing other things), 2011/10/14 15:48:42]

This has to be removed. 
This code is used is single-threaded testing to cleanup.  I won't upload this
change until I have code reviews from folks (since I put a lot of comments in
the left pane when I sent out for reviews)
Jim

   // We must be single threaded... but be careful anyway.
   if (!StartTracking(false))
     return;
   ThreadData* thread_data_list;
   {
     base::AutoLock lock(list_lock_);
     thread_data_list = first_;
     first_ = NULL;
   }
 
@@ skipping 35 matching lines @@
 }
 
 Snapshot::Snapshot(const BirthOnThread& birth_on_thread, int count)
     : birth_(&birth_on_thread),
       death_thread_(NULL),
       death_data_(DeathData(count)) {
 }
 
 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);
 }
 
 void Snapshot::Add(const Snapshot& other) {
   death_data_.AddDeathData(other.death_data_);
 }
 
 //------------------------------------------------------------------------------
 // DataCollector
 
 DataCollector::DataCollector() {
   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;
        thread_data;
        thread_data = thread_data->next()) {
     ++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()) {
     Append(*thread_data);
   }
 }
 
 DataCollector::~DataCollector() {
 }
 
@@ skipping 78 matching lines @@
       base::StringAppendF(output, "All born in %s. ",
                           birth_files_.begin()->first.c_str());
     }
   }
 
   if (birth_threads_.size() > 1) {
     base::StringAppendF(output, "%" PRIuS " BirthingThreads. ",
                         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) {
     base::StringAppendF(output, "%" PRIuS " DeathThreads. ",
                         death_threads_.size());
   } 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.");
     }
   }
 
   if (birth_count_ > 1)
     base::StringAppendF(output, "Births=%d ", birth_count_);
 
   DeathData::Write(output);
 }
@@ skipping 24 matching lines @@
   }
   use_tiebreaker_for_sort_only_ = false;
   selector_ = NIL;
 }
 
 bool Comparator::operator()(const Snapshot& left,
                             const Snapshot& right) const {
   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:
       if (left.death_thread() != right.death_thread() &&
           left.DeathThreadName() !=
           right.DeathThreadName()) {
         if (!left.death_thread())
           return true;
         if (!right.death_thread())
           return false;
@@ skipping 31 matching lines @@
         return left.count() > right.count();  // Sort large at front of vector.
       break;
 
     case AVERAGE_DURATION:
       if (!left.count() || !right.count())
         break;
       if (left.AverageMsDuration() != right.AverageMsDuration())
         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;
   }
   if (tiebreaker_)
     return tiebreaker_->operator()(left, right);
   return false;
 }
 
 void Comparator::Sort(DataCollector::Collection* collection) const {
   std::sort(collection->begin(), collection->end(), *this);
 }
 
 bool Comparator::Equivalent(const Snapshot& left,
                             const Snapshot& right) const {
   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;
 
     case DEATH_THREAD:
       if (left.death_thread() != right.death_thread() &&
           left.DeathThreadName() != right.DeathThreadName())
         return false;
       break;
 
     case BIRTH_FILE:
       if (left.location().file_name() != right.location().file_name()) {
         int comp = strcmp(left.location().file_name(),
                           right.location().file_name());
         if (comp)
           return false;
       }
       break;
 
     case BIRTH_FUNCTION:
       if (left.location().function_name() != right.location().function_name()) {
         int comp = strcmp(left.location().function_name(),
                           right.location().function_name());
         if (comp)
           return false;
       }
       break;
 
     case COUNT:
-      if (left.count() != right.count())

[jar (doing other things), 2011/10/14 02:29:53]

I decided it was stupid to aggregate groups of results with similar durations or
counts (similar names, yeah; similar files, yeah; similar durations, nah).  We
just act as though all pairs of durations are equivalent (unworthy of
speparating into individual sub-groups.
distinct groups in HTML), and pass. 

-        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:
       break;
   }
   if (tiebreaker_ && !use_tiebreaker_for_sort_only_)
     return tiebreaker_->Equivalent(left, right);
   return true;
 }
 
 bool Comparator::Acceptable(const Snapshot& sample) const {
   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;
 
       case DEATH_THREAD:
         if (sample.DeathThreadName().find(required_) == std::string::npos)
           return false;
         break;
 
       case BIRTH_FILE:
@@ skipping 55 matching lines @@
 }
 
 void Comparator::ParseKeyphrase(const std::string& key_phrase) {
   typedef std::map<const std::string, Selector> KeyMap;
   static KeyMap key_map;
   static bool initialized = false;
   if (!initialized) {
     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;
   }
 
   std::string required;
   // Watch for: "sort_key=value" as we parse.
   size_t equal_offset = key_phrase.find('=', 0);
   if (key_phrase.npos != equal_offset) {
     // There is a value that must be matched for the data to display.
@@ skipping 15 matching lines @@
   for (size_t i = 0; i < query.size();) {
     size_t slash_offset = query.find('/', i);
     ParseKeyphrase(query.substr(i, slash_offset - i));
     if (query.npos == slash_offset)
       break;
     i = slash_offset + 1;
   }
 
   // Select subgroup ordering (if we want to display the subgroup)
   SetSubgroupTiebreaker(COUNT);
+  SetSubgroupTiebreaker(TOTAL_DURATION);
   SetSubgroupTiebreaker(AVERAGE_DURATION);
   SetSubgroupTiebreaker(BIRTH_THREAD);
   SetSubgroupTiebreaker(DEATH_THREAD);
   SetSubgroupTiebreaker(BIRTH_FUNCTION);
   SetSubgroupTiebreaker(BIRTH_FILE);
   SetSubgroupTiebreaker(BIRTH_LINE);
 
   return true;
 }
 
 bool Comparator::WriteSortGrouping(const Snapshot& sample,
                                        std::string* output) const {
   bool wrote_data = false;
   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;
 
     case DEATH_THREAD:
       if (sample.death_thread()) {
         base::StringAppendF(output, "All deleted on %s ",
                             sample.DeathThreadName().c_str());
       } else {
         output->append("All still alive ");
       }
@@ skipping 20 matching lines @@
   return wrote_data;
 }
 
 void Comparator::WriteSnapshot(const Snapshot& sample,
                                std::string* output) const {
   sample.death_data().Write(output);
   if (!(combined_selectors_ & BIRTH_THREAD) ||
       !(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),
                                   !(combined_selectors_ & BIRTH_FUNCTION),
                                   output);
 }
 
 }  // namespace tracked_objects