Delivering the FIRST_NONEMPTY_PAINT phase changing event to base/

base/tracked_objects.h

 // 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.
 
 #ifndef BASE_TRACKED_OBJECTS_H_
 #define BASE_TRACKED_OBJECTS_H_
 
 #include <map>
 #include <set>
 #include <stack>
@@ skipping 227 matching lines @@
 
 class BASE_EXPORT Births: public BirthOnThread {
  public:
   Births(const Location& location, const ThreadData& current);
 
   int birth_count() const;
 
   // 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_;
 
   DISALLOW_COPY_AND_ASSIGN(Births);
 };
 
 //------------------------------------------------------------------------------
 // Basic info summarizing multiple destructions of a tracked object with a
 // single birthplace (fixed Location).  Used both on specific threads, and also
@@ skipping 17 matching lines @@
 
   // Metrics accessors, used only for serialization and in tests.
   int count() const;
   int32 run_duration_sum() const;
   int32 run_duration_max() const;
   int32 run_duration_sample() const;
   int32 queue_duration_sum() const;
   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:
   // 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).
   int count_;
   // Basic tallies, used to compute averages.
   int32 run_duration_sum_;
   int32 queue_duration_sum_;
@@ skipping 66 matching lines @@
   enum Status {
     UNINITIALIZED,              // PRistine, link-time state before running.
     DORMANT_DURING_TESTS,       // Only used during testing.
     DEACTIVATED,                // No longer recording profiling.
     PROFILING_ACTIVE,           // Recording profiles (no parent-child links).
     PROFILING_CHILDREN_ACTIVE,  // Fully active, recording parent-child links.
     STATUS_LAST = PROFILING_CHILDREN_ACTIVE
   };
 
   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;
 
   // Initialize the current thread context with a new instance of ThreadData.
   // This is used by all threads that have names, and should be explicitly
   // set *before* any births on the threads have taken place.  It is generally
   // only used by the message loop, which has a well defined thread name.
   static void InitializeThreadContext(const std::string& suggested_name);
 
   // Using Thread Local Store, find the current instance for collecting data.
   // If an instance does not exist, construct one (and remember it for use on
   // this thread.
   // This may return NULL if the system is disabled for any reason.
   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 OnProfilingPhaseCompletion(int profiling_phase);
 
   // Finds (or creates) a place to count births from the given location in this
   // thread, and increment that tally.
   // TallyABirthIfActive will returns NULL if the birth cannot be tallied.
   static Births* TallyABirthIfActive(const Location& location);
 
   // Records the end of a timed run of an object.  The |completed_task| contains
   // a pointer to a Births, the time_posted, and a delayed_start_time if any.
   // The |start_of_run| indicates when we started to perform the run of the
   // task.  The delayed_start_time is non-null for tasks that were posted as
   // 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.
   static void TallyRunOnNamedThreadIfTracking(
       const base::TrackingInfo& completed_task,
       const TaskStopwatch& stopwatch);
 
   // Record the end of a timed run of an object.  The |birth| is the record for
   // the instance, the |time_posted| records that instant, which is presumed to
   // be when the task was posted into a queue to run on a worker thread.
   // The |start_of_run| is when the worker thread started to perform the run of
   // 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);
+  // We don't modify anything in |birth| in this method. We store the pointer as

[Alexei Svitkine (slow), 2015/03/30 22:05:21]

Nit: Rephrase without "we". Same below.

[vadimt, 2015/04/01 00:45:40]

N/A anymore.

+  // a key in death_map_, and later, on a phase change event, will call
+  // SubtractBirths() on it.

[Alexei Svitkine (slow), 2015/03/30 22:05:21]

I think this comment needs to discuss the ownership guarantees of |birth| the
caller is expected to make for this to be safe.

I guess the assumption is that the birth should be something returned by
ThreadData::TallyABirthIfActive() and thus owned by ThreadData? If so, this
should be mentioned - as this API allows calling with any birth. In fact, I also
suggest adding a DCHECK_EQ(birth,
ThreadData::TallyABirthIfActive(birth->location()); to validate the contract.

Also, shouldn't the param be called |births| rather than birth?

Same for the other function below.

[vadimt, 2015/04/01 00:45:40]

Renamed |birth| to |births|.

On "the ownership guarantees of |birth| the caller is expected to make for this
to be safe.":

Thanks for raising this!

Long story short, I realized I had a race condition. I undone removing consts
from Births. Resetting death counts become more complicated, and now requires 2
passes: 1 to set death counts to 0 under their thread' locks, and the other - to
subtract death counts from birth counts under correct locks.

Note that a thread, under its lock, can modify birth counts from its birth map
and death counts from its death map. It cannot modify birth counts from its
death map (and that's why we have const there).

Also note that a Births instance, which is a value in the birth map of one
thread, can be a (const) key in the death map of another thread (which means
that the task was posted by one thread to the other).

We'd have less complexities if we could eliminate birth counts. They are
necessary for little-known and unused feature called "Still_Alive tasks". But I
couldn't prove to jar@ that we should remove it, so, we are keeping birth counts
in this CL.

+  static void TallyRunOnWorkerThreadIfTracking(const TrackedTime& time_posted,
+                                               const TaskStopwatch& stopwatch,
+                                               Births* birth);
 
   // Record the end of execution in region, generally corresponding to a scope
   // being exited.
-  static void TallyRunInAScopedRegionIfTracking(const Births* birth,
-                                                const TaskStopwatch& stopwatch);
+  // We don't modify anything in |birth| in this method. We store the pointer as
+  // a key in death_map_, and later, on a phase change event, will call
+  // SubtractBirths() on it.
+  static void TallyRunInAScopedRegionIfTracking(const TaskStopwatch& stopwatch,
+                                                Births* birth);
 
   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.
   static bool Initialize();
 
   // Sets internal status_.
   // If |status| is false, then status_ is set to DEACTIVATED.
   // If |status| is true, then status_ is set to, PROFILING_ACTIVE, or
@@ skipping 74 matching lines @@
   static ThreadData* first();
 
   // Iterate through the null terminated list of ThreadData instances.
   ThreadData* next() const;
 
 
   // In this thread's data, record a new birth.
   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);
+  // We don't modify anything in |birth| in this method. We store the pointer as
+  // a key in death_map_, and later, on a phase change event, will call
+  // SubtractBirths() on it.
+  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
   // 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,
+  // 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);
 
   // 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);
 
   // This method should be called when a worker thread terminates, so that we
   // can save all the thread data into a cache of reusable ThreadData instances.
   void OnThreadTerminationCleanup();
@@ skipping 47 matching lines @@
 
   // Protection for access to all_thread_data_list_head_, and to
   // unregistered_thread_data_pool_.  This lock is leaked at shutdown.
   // The lock is very infrequently used, so we can afford to just make a lazy
   // instance and be safe.
   static base::LazyInstance<base::Lock>::Leaky list_lock_;
 
   // 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).
   ThreadData* next_;
 
   // Pointer to another ThreadData instance for a Worker-Thread that has been
   // retired (its thread was terminated).  This value is non-NULL only for a
   // retired ThreadData associated with a Worker-Thread.
   ThreadData* next_retired_worker_;
 
@@ skipping 158 matching lines @@
   ProcessDataSnapshot();
   ~ProcessDataSnapshot();
 
   PhasedProcessDataSnapshotMap phased_process_data_snapshots;
   base::ProcessId process_id;
 };
 
 }  // namespace tracked_objects
 
 #endif  // BASE_TRACKED_OBJECTS_H_