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>
 #include <string>
 #include <utility>
 #include <vector>
 
 #include "base/base_export.h"
 #include "base/basictypes.h"
 #include "base/containers/hash_tables.h"
 #include "base/gtest_prod_util.h"
 #include "base/lazy_instance.h"
 #include "base/location.h"
 #include "base/process/process_handle.h"
 #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 {
 struct TrackingInfo;
 }
 
 // TrackedObjects provides a database of stats about objects (generally Tasks)
 // that are tracked.  Tracking means their birth, death, duration, birth thread,
 // death thread, and birth place are recorded.  This data is carefully spread
 // across a series of objects so that the counts and times can be rapidly
@@ skipping 212 matching lines @@
   void RecordBirth();
 
  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
-// 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, bout this would

[Ilya Sherman, 2015/04/15 00:37:35]

nit: s/bout/but

[vadimt, 2015/04/15 19:14:31]

Done.

+  // create a loop since DeathData indirectly refers DeathDataSnapshot. Passing
+  // wrapper structure as a param or using an empty constructor for snapshotting

[Ilya Sherman, 2015/04/15 00:37:35]

nit: "Passing wrapper structure" -> "Passing a wrapper structure"

[vadimt, 2015/04/15 19:14:31]

Done.

+  // DeathData would be less efficient.

[Ilya Sherman, 2015/04/15 00:37:35]

It looks like DeathData only refers to this class via a pointer.  Why not
forward-declare these classes, so that DeathData can refer to them, but define
them below?

[vadimt, 2015/04/15 19:14:32]

We spent lots of effort breaking the loop between DeathDataSnapshot and
DeathData (asvitkine@ preference).
I think it would be in the spirit of these changes to now order the classes
correspondingly, otherwise, what was the point of breaking the loop?

+  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 delta between this snapshot and an earlier snapshot of the same
+  // task |older| and assigns it back to this object.
+  void SubtractOlderSnapshot(const DeathDataSnapshot& older);
+
+  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,
+                         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.
+  DeathDataPhaseSnapshot* prev;

[Ilya Sherman, 2015/04/15 00:37:35]

Why not use a scoped_ptr, both here and below?

[vadimt, 2015/04/15 19:14:31]

DeathData owns the whole list of DeathDataPhaseSnapshot's and deletes everything
in its destructor in a loop.
However, we happen to create copies of DeathDataPhaseSnapshot, and we don't want
to deleted the list pointed by 'prev' when the copy is destructed.

[Ilya Sherman, 2015/04/17 02:14:26]

Hmm, where do we create copies?  This seems like a somewhat fragile memory
management situation.  What guarantees that the memory won't be deleted out from
under the copy?

[vadimt, 2015/04/17 16:15:50]

Terminological clarification: not quite a 'copy', but an instance of
DeathDataPhaseSnapshot gets created in ThreadData::SnapshotMaps.

DeathDatas are simply leaked in normal Chrome, i.e. ThreadDatas are never
deleted. This is by design, or at least, how it always was. Note that DeathDatas
are never removed from the death map.

In tests, ThreadData, and all DeathDatas that it owns inside its death map, are
deleted at the end of each test. Just in case, I added a thread check in
~ThreadData to totally rule out races now and for all.

+};
+
+//------------------------------------------------------------------------------
+// Information about deaths of a task on a given thread, called "death thread".
+// Access to members of this class is never protected by the 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 snapshots and phase change notifications 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.

[Ilya Sherman, 2015/04/15 00:37:35]

nit: Please omit this comment.

[vadimt, 2015/04/15 19:14:31]

Done.

   DeathData();
 

[Ilya Sherman, 2015/04/15 00:37:35]

nit: Please omit this newline.

[vadimt, 2015/04/15 19:14:31]

Done.

-  // 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();
 
   // Update stats for a task destruction (death) that had a Run() time of
   // |duration|, and has had a queueing delay of |queue_duration|.
   void RecordDeath(const int32 queue_duration,
                    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;
   int32 run_duration_sample() const;
   int32 queue_duration_sum() const;
   int32 queue_duration_max() const;
   int32 queue_duration_sample() 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_;
+
+  // 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.
+  DeathDataPhaseSnapshot* last_phase_snapshot_;
+
+  DISALLOW_COPY_AND_ASSIGN(DeathData);
 };
 
 //------------------------------------------------------------------------------
-// A "snapshotted" representation of the DeathData class.
-
-struct BASE_EXPORT DeathDataSnapshot {
-  DeathDataSnapshot();
-  explicit DeathDataSnapshot(const DeathData& death_data);
-  ~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;
-};
-
-//------------------------------------------------------------------------------
 // A temporary collection of data that can be sorted and summarized.  It is
 // gathered (carefully) from many threads.  Instances are held in arrays and
 // processed, filtered, and rendered.
 // The source of this data was collected on many threads, and is asynchronously
 // changing.  The data in this instance is not asynchronously changing.
 
 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;
 };
 
 //------------------------------------------------------------------------------
 // For each thread, we have a ThreadData that stores all tracking info generated
 // on this thread.  This prevents the need for locking as data accumulates.
 // We use ThreadLocalStorage to quickly identfy the current ThreadData context.
 // We also have a linked list of ThreadData instances, and that list is used to
 // harvest data from all existing instances.
@@ skipping 32 matching lines @@
   // 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 OnProfilingPhaseCompleted(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,
+  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.
   static bool Initialize();
 
   // Sets internal status_.
   // If |status| is false, then status_ is set to DEACTIVATED.
@@ skipping 51 matching lines @@
   // TODO(jar): Make this a friend in DEBUG only, so that the optimizer has a
   // better change of optimizing (inlining? etc.) private methods (knowing that
   // there will be no need for an external entry point).
   friend class TrackedObjectsTest;
   FRIEND_TEST_ALL_PREFIXES(TrackedObjectsTest, MinimalStartupShutdown);
   FRIEND_TEST_ALL_PREFIXES(TrackedObjectsTest, TinyStartupShutdown);
   FRIEND_TEST_ALL_PREFIXES(TrackedObjectsTest, ParentChildTest);
 
   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);
 
   // Message loop based construction should provide a name.
   explicit ThreadData(const std::string& suggested_name);
 
   ~ThreadData();
 
   // Push this instance to the head of all_thread_data_list_head_, linking it to
   // the previous head.  This is performed after each construction, and leaves
   // the instance permanently on that list.
   void PushToHeadOfList();
 
   // (Thread safe) Get start of list of all ThreadData instances using the lock.
   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,
+  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
+  // |return_of_snapshot_hrenina|. Also updates the |birth_counts| tally for

[Ilya Sherman, 2015/04/15 00:37:35]

return_of_snapshot_hrenina?

[vadimt, 2015/04/15 19:14:31]

Done :)
One of the primordial names leaked into the final code :)

+  // 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 OnProfilingPhaseCompletionOnThread(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);
 
   // 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();
 
   // Cleans up data structures, and returns statics to near pristine (mostly
   // uninitialized) state.  If there is any chance that other threads are still
@@ skipping 41 matching lines @@
   // we've either transitioned out of UNINITIALIZED, or into that state.  This
   // value is only accessed while the list_lock_ is held.
   static int incarnation_counter_;
 
   // 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_;
 
+  // Checker that all snapshots and phase change notifications happen in the
+  // same thread.
+  static base::LazyInstance<base::ThreadChecker>::Leaky
+      snapshot_thread_checker_;
+
   // 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
   // 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
@@ skipping 154 matching lines @@
 
 //------------------------------------------------------------------------------
 // A snapshotted representation of the list of ThreadData objects for a process,
 // for all profiling phases, including the current one.
 
 struct BASE_EXPORT ProcessDataSnapshot {
  public:
   ProcessDataSnapshot();
   ~ProcessDataSnapshot();
 
-  PhasedProcessDataSnapshotMap phased_process_data_snapshots;
+  PhasedProcessDataSnapshotMap phased_snapshots;
   base::ProcessId process_id;
 };
 
 }  // namespace tracked_objects
 
 #endif  // BASE_TRACKED_OBJECTS_H_