Delivering the FIRST_NONEMPTY_PAINT phase changing event to base/

base/tracked_objects.h

Index: base/tracked_objects.h
diff --git a/base/tracked_objects.h b/base/tracked_objects.h
index 00bdb2dfc5392631cfe0206fc08107406f073fe3..ee580e45ede3ae53f5b5acb253b27e9a092a2194 100644
--- a/base/tracked_objects.h
+++ b/base/tracked_objects.h
@@ -245,6 +245,9 @@ class BASE_EXPORT Births: public BirthOnThread {
   // When we have a birth we update the count for this birthplace.
   void RecordBirth();
 
+  // Subtracts a value from the birth count.
+  void SubtractBirths(int count);
+
  private:
   // The number of births on this thread for our location_.
   int birth_count_;
@@ -282,7 +285,7 @@ class BASE_EXPORT DeathData {
   int32 queue_duration_max() const;
   int32 queue_duration_sample() const;
 
-  // Reset all tallies to zero. This is used as a hack on realtime data.
+  // Reset all tallies to zero.
   void Clear();
 
  private:
@@ -369,7 +372,7 @@ class BASE_EXPORT ThreadData {
   };
 
   typedef std::map<Location, Births*> BirthMap;
-  typedef std::map<const Births*, DeathData> DeathMap;
+  typedef std::map<Births*, DeathData> DeathMap;
   typedef std::pair<const Births*, const Births*> ParentChildPair;
   typedef std::set<ParentChildPair> ParentChildSet;
   typedef std::stack<const Births*> ParentStack;
@@ -387,8 +390,13 @@ 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|.

[Alexei Svitkine (slow), 2015/03/30 18:31:21]

I think I've made this comment before and don't remember the reply, but it's
still confusing to me (which means there's room to improve this even if there's
a good reason to keep the way it is).

But if this is filling the snapshot for all profile phases, why is the
|current_profiling_phase| needed? Is it because this tells the profiler that
this phase ended or something for keeping track of for later? If so, this should
be called out in the comment (and maybe the function name should change to
reflect that in that case).

[vadimt, 2015/03/30 20:36:05]

First, please rest assured that no comment gets ignored and quietly forgotten :)

|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 central
process. (If the process didn't receive phase-changing events after it was
started, it has no information to deduce the current phase from).

Added a comment.

+  static void Snapshot(int current_profiling_phase,
+                       ProcessDataSnapshot* process_data_snapshot);
+
+  // Called when the current profiling phase, identified by |profiling_phase|,
+  // ends.
+  static void OnProfilingPhaseCompletion(int profiling_phase);
 
   // Finds (or creates) a place to count births from the given location in this
   // thread, and increment that tally.
@@ -414,14 +422,14 @@ 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,
-                                               const TrackedTime& time_posted,
-                                               const TaskStopwatch& stopwatch);
+  static void TallyRunOnWorkerThreadIfTracking(const TrackedTime& time_posted,
+                                               const TaskStopwatch& stopwatch,
+                                               Births* birth);

[Alexei Svitkine (slow), 2015/03/30 18:31:21]

Could you update comments for these two methods to mention what changes to
|birth| this can cause?

[vadimt, 2015/03/30 20:36:05]

Done.

 
   // Record the end of execution in region, generally corresponding to a scope
   // being exited.
-  static void TallyRunInAScopedRegionIfTracking(const Births* birth,
-                                                const TaskStopwatch& stopwatch);
+  static void TallyRunInAScopedRegionIfTracking(const TaskStopwatch& stopwatch,
+                                                Births* birth);
 
   const std::string& thread_name() const { return thread_name_; }
 
@@ -516,19 +524,23 @@ class BASE_EXPORT ThreadData {
   Births* TallyABirth(const Location& location);
 
   // Find a place to record a death on this thread.
-  void TallyADeath(const Births& birth,
-                   int32 queue_duration,
-                   const TaskStopwatch& stopwatch);
+  void TallyADeath(int32 queue_duration,
+                   const TaskStopwatch& stopwatch,
+                   Births* birth);
 
   // 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.
+  // track of the number of living instances of the task. If |reset| is true,
+  // remove all recorded deaths.
   static void SnapshotAllExecutedTasks(
+      bool reset,
       ProcessDataPhaseSnapshot* process_data_phase,
       BirthCountMap* birth_counts);
 
   // Fills |process_data_phase| with all the recursive results in our process.
+  // If |reset| is true, remove all recorded deaths.
   static void SnapshotCurrentPhase(
+      bool reset,
       ProcessDataPhaseSnapshot* process_data_phase);
 
   // Snapshots (under a lock) the profiled data for the tasks for this thread
@@ -536,14 +548,18 @@ class BASE_EXPORT ThreadData {
   // 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,
+  // births for the task that have not yet been balanced by a death. If |reset|
+  // is true, remove all recorded deaths.
+  void SnapshotExecutedTasks(bool reset,
+                             ProcessDataPhaseSnapshot* process_data_phase,
                              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,
+  // the map(s) from being reallocated while they are copied. If |reset| is
+  // true, remove all recorded deaths.
+  void SnapshotMaps(bool reset,
+                    BirthMap* birth_map,
                     DeathMap* death_map,
                     ParentChildSet* parent_child_set);
 
@@ -611,6 +627,10 @@ class BASE_EXPORT ThreadData {
   // We set status_ to SHUTDOWN when we shut down the tracking service.
   static Status status_;
 
+  // Process data snapshots for completed profiling phases.
+  static base::LazyInstance<PhasedProcessDataSnapshotMap>
+      completed_phases_snapshots_;
+
   // Link to next instance (null terminated list). Used to globally track all
   // registered instances (corresponds to all registered threads where we keep
   // data).