Delivering the FIRST_NONEMPTY_PAINT phase changing event to base/. This concludes the client-side “phased profiling” work.

I’m removing TODO items, and the DIFFS introduced by this should remind the reviewers that there are open questions that they wanted to re-raise, assuming the the CL doesn’t make it clear that the way it’s implemented is OK.

If there are still concerns, we should agree on one important thing before we resume the discussions: phase change delivery chain doesn’t go through metrics provider. It starts at the event source (FirstWebContentsProfiler), then goes to TrackingSynchronizer, then goes to base/. Once we agree on the design of this chain, it will make sense to go to more detailed discussions and open questions.
BUG=456354

Review URL: https://codereview.chromium.org/1021053003

Cr-Commit-Position: refs/heads/master@{#327143}

1 files changed, 156 insertions, 75 deletions

diff --git a/base/tracked_objects.h b/base/tracked_objects.h
index 9643265..0f55a07 100644
--- a/base/tracked_objects.h
+++ b/base/tracked_objects.h
@@ -22,6 +22,7 @@
 #include "base/profiler/alternate_timer.h"
 #include "base/profiler/tracked_time.h"
 #include "base/synchronization/lock.h"
+#include "base/threading/thread_checker.h"
 #include "base/threading/thread_local_storage.h"
 
 namespace base {
@@ -85,7 +86,7 @@ struct TrackingInfo;
 // threads there are, and how many Locations of construction there are.
 // Fortunately, we don't use memory that is the product of those two counts, but
 // rather we only need one Births instance for each thread that constructs an
-// instance at a Location. In many cases, instances are only created on one
+// instance at a Location.  In many cases, instances are only created on one
 // thread, so the memory utilization is actually fairly restrained.
 //
 // Lastly, when an instance is deleted, the final tallies of statistics are
@@ -106,14 +107,14 @@ struct TrackingInfo;
 // Each thread maintains a list of data items specific to that thread in a
 // ThreadData instance (for that specific thread only).  The two critical items
 // are lists of DeathData and Births instances.  These lists are maintained in
-// STL maps, which are indexed by Location. As noted earlier, we can compare
+// STL maps, which are indexed by Location.  As noted earlier, we can compare
 // locations very efficiently as we consider the underlying data (file,
 // function, line) to be atoms, and hence pointer comparison is used rather than
 // (slow) string comparisons.
 //
 // To provide a mechanism for iterating over all "known threads," which means
 // threads that have recorded a birth or a death, we create a singly linked list
-// of ThreadData instances. Each such instance maintains a pointer to the next
+// of ThreadData instances.  Each such instance maintains a pointer to the next
 // one.  A static member of ThreadData provides a pointer to the first item on
 // this global list, and access via that all_thread_data_list_head_ item
 // requires the use of the list_lock_.
@@ -148,13 +149,13 @@ struct TrackingInfo;
 // TaskSnapshot instances, so that such instances can be sorted and
 // aggregated (and remain frozen during our processing).
 //
-// Profiling consists of phases. The concrete phase in the sequence of phases is
-// identified by its 0-based index.
+// Profiling consists of phases.  The concrete phase in the sequence of phases
+// is identified by its 0-based index.
 //
 // The ProcessDataPhaseSnapshot struct is a serialized representation of the
-// list of ThreadData objects for a process for a concrete profiling phase. It
+// list of ThreadData objects for a process for a concrete profiling phase.  It
 // holds a set of TaskSnapshots and tracks parent/child relationships for the
-// executed tasks. The statistics in a snapshot are gathered asynhcronously
+// executed tasks.  The statistics in a snapshot are gathered asynhcronously
 // relative to their ongoing updates.
 // It is possible, though highly unlikely, that stats could be incorrectly
 // recorded by this process (all data is held in 32 bit ints, but we are not
@@ -254,19 +255,80 @@ class BASE_EXPORT Births: public BirthOnThread {
 };
 
 //------------------------------------------------------------------------------
-// Basic info summarizing multiple destructions of a tracked object with a
-// single birthplace (fixed Location).  Used both on specific threads, and also
-// in snapshots when integrating assembled data.
+// A "snapshotted" representation of the DeathData class.
+
+struct BASE_EXPORT DeathDataSnapshot {
+  DeathDataSnapshot();
+
+  // Constructs the snapshot from individual values.
+  // The alternative would be taking a DeathData parameter, but this would
+  // create a loop since DeathData indirectly refers DeathDataSnapshot.  Passing
+  // a wrapper structure as a param or using an empty constructor for
+  // snapshotting DeathData would be less efficient.
+  DeathDataSnapshot(int count,
+                    int32 run_duration_sum,
+                    int32 run_duration_max,
+                    int32 run_duration_sample,
+                    int32 queue_duration_sum,
+                    int32 queue_duration_max,
+                    int32 queue_duration_sample);
+  ~DeathDataSnapshot();
+
+  // Calculates and returns the delta between this snapshot and an earlier
+  // snapshot of the same task |older|.
+  DeathDataSnapshot Delta(const DeathDataSnapshot& older) const;
+
+  int count;
+  int32 run_duration_sum;
+  int32 run_duration_max;
+  int32 run_duration_sample;
+  int32 queue_duration_sum;
+  int32 queue_duration_max;
+  int32 queue_duration_sample;
+};
+
+//------------------------------------------------------------------------------
+// A "snapshotted" representation of the DeathData for a particular profiling
+// phase.  Used as an element of the list of phase snapshots owned by DeathData.
+
+struct DeathDataPhaseSnapshot {
+  DeathDataPhaseSnapshot(int profiling_phase,
+                         int count,
+                         int32 run_duration_sum,
+                         int32 run_duration_max,
+                         int32 run_duration_sample,
+                         int32 queue_duration_sum,
+                         int32 queue_duration_max,
+                         int32 queue_duration_sample,
+                         const DeathDataPhaseSnapshot* prev);
+
+  // Profiling phase at which completion this snapshot was taken.
+  int profiling_phase;
+
+  // Death data snapshot.
+  DeathDataSnapshot death_data;
+
+  // Pointer to a snapshot from the previous phase.
+  const DeathDataPhaseSnapshot* prev;
+};
+
+//------------------------------------------------------------------------------
+// Information about deaths of a task on a given thread, called "death thread".
+// Access to members of this class is never protected by a lock.  The fields
+// are accessed in such a way that corruptions resulting from race conditions
+// are not significant, and don't accumulate as a result of multiple accesses.
+// All invocations of DeathData::OnProfilingPhaseCompleted and
+// ThreadData::SnapshotMaps (which takes DeathData snapshot) in a given process
+// must be called from the same thread. It doesn't matter what thread it is, but
+// it's important the same thread is used as a snapshot thread during the whole
+// process lifetime.  All fields except sample_probability_count_ can be
+// snapshotted.
 
 class BASE_EXPORT DeathData {
  public:
-  // Default initializer.
   DeathData();
-
-  // When deaths have not yet taken place, and we gather data from all the
-  // threads, we create DeathData stats that tally the number of births without
-  // a corresponding death.
-  explicit DeathData(int count);
+  DeathData(const DeathData& other);
+  ~DeathData();
 
   // Update stats for a task destruction (death) that had a Run() time of
   // |duration|, and has had a queueing delay of |queue_duration|.
@@ -274,7 +336,8 @@ class BASE_EXPORT DeathData {
                    const int32 run_duration,
                    const uint32 random_number);
 
-  // Metrics accessors, used only for serialization and in tests.
+  // Metrics and past snapshots accessors, used only for serialization and in
+  // tests.
   int count() const;
   int32 run_duration_sum() const;
   int32 run_duration_max() const;
@@ -282,43 +345,47 @@ class BASE_EXPORT DeathData {
   int32 queue_duration_sum() const;
   int32 queue_duration_max() const;
   int32 queue_duration_sample() const;
+  const DeathDataPhaseSnapshot* last_phase_snapshot() const;
 
-  // Reset all tallies to zero. This is used as a hack on realtime data.
-  void Clear();
+  // Called when the current profiling phase, identified by |profiling_phase|,
+  // ends.
+  // Must be called only on the snapshot thread.
+  void OnProfilingPhaseCompleted(int profiling_phase);
 
  private:
   // Members are ordered from most regularly read and updated, to least
   // frequently used.  This might help a bit with cache lines.
   // Number of runs seen (divisor for calculating averages).
+  // Can be incremented only on the death thread.
   int count_;
-  // Basic tallies, used to compute averages.
+
+  // Count used in determining probability of selecting exec/queue times from a
+  // recorded death as samples.
+  // Gets incremented only on the death thread, but can be set to 0 by
+  // OnProfilingPhaseCompleted() on the snapshot thread.
+  int sample_probability_count_;
+
+  // Basic tallies, used to compute averages.  Can be incremented only on the
+  // death thread.
   int32 run_duration_sum_;
   int32 queue_duration_sum_;
   // Max values, used by local visualization routines.  These are often read,
-  // but rarely updated.
+  // but rarely updated.  The max values get assigned only on the death thread,
+  // but these fields can be set to 0 by OnProfilingPhaseCompleted() on the
+  // snapshot thread.
   int32 run_duration_max_;
   int32 queue_duration_max_;
   // Samples, used by crowd sourcing gatherers.  These are almost never read,
-  // and rarely updated.
+  // and rarely updated.  They can be modified only on the death thread.
   int32 run_duration_sample_;
   int32 queue_duration_sample_;
-};
 
-//------------------------------------------------------------------------------
-// A "snapshotted" representation of the DeathData class.
-
-struct BASE_EXPORT DeathDataSnapshot {
-  DeathDataSnapshot();
-  explicit DeathDataSnapshot(const DeathData& death_data);
-  ~DeathDataSnapshot();
+  // Snapshot of this death data made at the last profiling phase completion, if
+  // any.  DeathData owns the whole list starting with this pointer.
+  // Can be accessed only on the snapshot thread.
+  const DeathDataPhaseSnapshot* last_phase_snapshot_;
 
-  int count;
-  int32 run_duration_sum;
-  int32 run_duration_max;
-  int32 run_duration_sample;
-  int32 queue_duration_sum;
-  int32 queue_duration_max;
-  int32 queue_duration_sample;
+  DISALLOW_ASSIGN(DeathData);
 };
 
 //------------------------------------------------------------------------------
@@ -330,12 +397,14 @@ struct BASE_EXPORT DeathDataSnapshot {
 
 struct BASE_EXPORT TaskSnapshot {
   TaskSnapshot();
-  TaskSnapshot(const BirthOnThread& birth,
-               const DeathData& death_data,
+  TaskSnapshot(const BirthOnThreadSnapshot& birth,
+               const DeathDataSnapshot& death_data,
                const std::string& death_thread_name);
   ~TaskSnapshot();
 
   BirthOnThreadSnapshot birth;
+  // Delta between death data for a thread for a certain profiling phase and the
+  // snapshot for the pervious phase, if any.  Otherwise, just a snapshot.
   DeathDataSnapshot death_data;
   std::string death_thread_name;
 };
@@ -388,8 +457,21 @@ class BASE_EXPORT ThreadData {
   static ThreadData* Get();
 
   // Fills |process_data_snapshot| with phased snapshots of all profiling
-  // phases, including the current one.
-  static void Snapshot(ProcessDataSnapshot* process_data_snapshot);
+  // phases, including the current one, identified by |current_profiling_phase|.
+  // |current_profiling_phase| is necessary because a child process can start
+  // after several phase-changing events, so it needs to receive the current
+  // phase number from the browser process to fill the correct entry for the
+  // current phase in the |process_data_snapshot| map.
+  static void Snapshot(int current_profiling_phase,
+                       ProcessDataSnapshot* process_data_snapshot);
+
+  // Called when the current profiling phase, identified by |profiling_phase|,
+  // ends.
+  // |profiling_phase| is necessary because a child process can start after
+  // several phase-changing events, so it needs to receive the phase number from
+  // the browser process to fill the correct entry in the
+  // completed_phases_snapshots_ map.
+  static void OnProfilingPhaseCompleted(int profiling_phase);
 
   // Finds (or creates) a place to count births from the given location in this
   // thread, and increment that tally.
@@ -403,7 +485,7 @@ class BASE_EXPORT ThreadData {
   // delayed tasks, and it indicates when the task should have run (i.e., when
   // it should have posted out of the timer queue, and into the work queue.
   // The |end_of_run| was just obtained by a call to Now() (just after the task
-  // finished). It is provided as an argument to help with testing.
+  // finished).  It is provided as an argument to help with testing.
   static void TallyRunOnNamedThreadIfTracking(
       const base::TrackingInfo& completed_task,
       const TaskStopwatch& stopwatch);
@@ -415,19 +497,19 @@ class BASE_EXPORT ThreadData {
   // the task.
   // The |end_of_run| was just obtained by a call to Now() (just after the task
   // finished).
-  static void TallyRunOnWorkerThreadIfTracking(const Births* birth,
+  static void TallyRunOnWorkerThreadIfTracking(const Births* births,
                                                const TrackedTime& time_posted,
                                                const TaskStopwatch& stopwatch);
 
   // Record the end of execution in region, generally corresponding to a scope
   // being exited.
-  static void TallyRunInAScopedRegionIfTracking(const Births* birth,
+  static void TallyRunInAScopedRegionIfTracking(const Births* births,
                                                 const TaskStopwatch& stopwatch);
 
   const std::string& thread_name() const { return thread_name_; }
 
   // Initializes all statics if needed (this initialization call should be made
-  // while we are single threaded). Returns false if unable to initialize.
+  // while we are single threaded).  Returns false if unable to initialize.
   static bool Initialize();
 
   // Sets internal status_.
@@ -452,7 +534,7 @@ class BASE_EXPORT ThreadData {
   // on.  This is currently a compiled option, atop TrackingStatus().
   static bool TrackingParentChildStatus();
 
-  // Marks a start of a tracked run. It's super fast when tracking is disabled,
+  // Marks a start of a tracked run.  It's super fast when tracking is disabled,
   // and has some internal side effects when we are tracking, so that we can
   // deduce the amount of time accumulated outside of execution of tracked runs.
   // The task that will be tracked is passed in as |parent| so that parent-child
@@ -493,6 +575,9 @@ class BASE_EXPORT ThreadData {
 
   typedef std::map<const BirthOnThread*, int> BirthCountMap;
 
+  typedef std::vector<std::pair<const Births*, DeathDataPhaseSnapshot>>
+      DeathsSnapshot;
+
   // Worker thread construction creates a name since there is none.
   explicit ThreadData(int thread_number);
 
@@ -517,37 +602,33 @@ class BASE_EXPORT ThreadData {
   Births* TallyABirth(const Location& location);
 
   // Find a place to record a death on this thread.
-  void TallyADeath(const Births& birth,
+  void TallyADeath(const Births& births,
                    int32 queue_duration,
                    const TaskStopwatch& stopwatch);
 
-  // Snapshot (under a lock) the profiled data for the tasks in each ThreadData
-  // instance.  Also updates the |birth_counts| tally for each task to keep
-  // track of the number of living instances of the task.
-  static void SnapshotAllExecutedTasks(
-      ProcessDataPhaseSnapshot* process_data_phase,
-      BirthCountMap* birth_counts);
-
-  // Fills |process_data_phase| with all the recursive results in our process.
-  static void SnapshotCurrentPhase(
-      ProcessDataPhaseSnapshot* process_data_phase);
-
   // Snapshots (under a lock) the profiled data for the tasks for this thread
-  // and writes all of the executed tasks' data -- i.e. the data for the tasks
-  // with with entries in the death_map_ -- into |process_data_phase|.  Also
-  // updates the |birth_counts| tally for each task to keep track of the number
-  // of living instances of the task -- that is, each task maps to the number of
-  // births for the task that have not yet been balanced by a death.
-  void SnapshotExecutedTasks(ProcessDataPhaseSnapshot* process_data_phase,
+  // and writes all of the executed tasks' data -- i.e. the data for all
+  // profiling phases (including the current one: |current_profiling_phase|) for
+  // the tasks with with entries in the death_map_ -- into |phased_snapshots|.
+  // Also updates the |birth_counts| tally for each task to keep track of the
+  // number of living instances of the task -- that is, each task maps to the
+  // number of births for the task that have not yet been balanced by a death.
+  void SnapshotExecutedTasks(int current_profiling_phase,
+                             PhasedProcessDataSnapshotMap* phased_snapshots,
                              BirthCountMap* birth_counts);
 
   // Using our lock, make a copy of the specified maps.  This call may be made
   // on  non-local threads, which necessitate the use of the lock to prevent
   // the map(s) from being reallocated while they are copied.
-  void SnapshotMaps(BirthMap* birth_map,
-                    DeathMap* death_map,
+  void SnapshotMaps(int profiling_phase,
+                    BirthMap* birth_map,
+                    DeathsSnapshot* deaths,
                     ParentChildSet* parent_child_set);
 
+  // Called for this thread when the current profiling phase, identified by
+  // |profiling_phase|, ends.
+  void OnProfilingPhaseCompletedOnThread(int profiling_phase);
+
   // This method is called by the TLS system when a thread terminates.
   // The argument may be NULL if this thread has never tracked a birth or death.
   static void OnThreadTermination(void* thread_data);
@@ -578,7 +659,7 @@ class BASE_EXPORT ThreadData {
   // We use thread local store to identify which ThreadData to interact with.
   static base::ThreadLocalStorage::StaticSlot tls_index_;
 
-  // List of ThreadData instances for use with worker threads. When a worker
+  // List of ThreadData instances for use with worker threads.  When a worker
   // thread is done (terminated), we push it onto this list.  When a new worker
   // thread is created, we first try to re-use a ThreadData instance from the
   // list, and if none are available, construct a new one.
@@ -595,7 +676,7 @@ class BASE_EXPORT ThreadData {
   static int worker_thread_data_creation_count_;
 
   // The number of times TLS has called us back to cleanup a ThreadData
-  // instance. This is only accessed while list_lock_ is held.
+  // instance.  This is only accessed while list_lock_ is held.
   static int cleanup_count_;
 
   // Incarnation sequence number, indicating how many times (during unittests)
@@ -612,7 +693,7 @@ class BASE_EXPORT ThreadData {
   // We set status_ to SHUTDOWN when we shut down the tracking service.
   static Status status_;
 
-  // Link to next instance (null terminated list). Used to globally track all
+  // Link to next instance (null terminated list).  Used to globally track all
   // registered instances (corresponds to all registered threads where we keep
   // data).
   ThreadData* next_;
@@ -644,7 +725,7 @@ class BASE_EXPORT ThreadData {
   // locking before reading it.
   DeathMap death_map_;
 
-  // A set of parents that created children tasks on this thread. Each pair
+  // A set of parents that created children tasks on this thread.  Each pair
   // corresponds to potentially non-local Births (location and thread), and a
   // local Births (that took place on this thread).
   ParentChildSet parent_child_set_;
@@ -657,7 +738,7 @@ class BASE_EXPORT ThreadData {
   // writing is only done from this thread.
   mutable base::Lock map_lock_;
 
-  // The stack of parents that are currently being profiled. This includes only
+  // The stack of parents that are currently being profiled.  This includes only
   // tasks that have started a timer recently via PrepareForStartOfRun(), but
   // not yet concluded with a NowForEndOfRun().  Usually this stack is one deep,
   // but if a scoped region is profiled, or <sigh> a task runs a nested-message
@@ -688,7 +769,7 @@ class BASE_EXPORT ThreadData {
 
 //------------------------------------------------------------------------------
 // Stopwatch to measure task run time or simply create a time interval that will
-// be subtracted from the current most nested task's run time. Stopwatches
+// be subtracted from the current most nested task's run time.  Stopwatches
 // coordinate with the stopwatches in which they are nested to avoid
 // double-counting nested tasks run times.
 
@@ -736,12 +817,12 @@ class BASE_EXPORT TaskStopwatch {
   TaskStopwatch* parent_;
 
 #if DCHECK_IS_ON()
-  // State of the stopwatch. Stopwatch is first constructed in a created state
+  // State of the stopwatch.  Stopwatch is first constructed in a created state
   // state, then is optionally started/stopped, then destructed.
   enum { CREATED, RUNNING, STOPPED } state_;
 
   // Currently running stopwatch that is directly nested in this one, if such
-  // stopwatch exists. NULL otherwise.
+  // stopwatch exists.  NULL otherwise.
   TaskStopwatch* child_;
 #endif
 };
@@ -783,7 +864,7 @@ struct BASE_EXPORT ProcessDataSnapshot {
   ProcessDataSnapshot();
   ~ProcessDataSnapshot();
 
-  PhasedProcessDataSnapshotMap phased_process_data_snapshots;
+  PhasedProcessDataSnapshotMap phased_snapshots;
   base::ProcessId process_id;
 };