Make stack sampling profiler sample beyond startup.

base/profiler/stack_sampling_profiler.cc

 // Copyright 2015 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/profiler/stack_sampling_profiler.h"
 
 #include <algorithm>
 #include <map>
 #include <utility>
 
@@ skipping 121 matching lines @@
     static void ShutdownAssumingIdle(bool simulate_intervening_add);
 
    private:
     // Calls the sampling threads ShutdownTask and then signals an event.
     static void ShutdownTaskAndSignalEvent(SamplingThread* sampler,
                                            int add_events,
                                            WaitableEvent* event);
   };
 
   struct CollectionContext {
-    CollectionContext(PlatformThreadId target,
+    CollectionContext(int collection_id,
+                      PlatformThreadId target,
                       const SamplingParams& params,
                       const CompletedCallback& callback,
                       WaitableEvent* finished,
                       std::unique_ptr<NativeStackSampler> sampler)
-        : collection_id(next_collection_id_.GetNext()),
+        : collection_id(collection_id),
           target(target),
           params(params),
           callback(callback),
           finished(finished),
           native_sampler(std::move(sampler)) {}
     ~CollectionContext() {}
 
     // An identifier for this collection, used to uniquely identify it to
     // outside interests.
     const int collection_id;

[Mike Wittman, 2017/07/06 17:25:52]

We probably should rename this to something like "profiler_id" since it's
effectively identifying the requester of the profiling across multiple
collections at this point.

[Alexei Svitkine (slow), 2017/07/06 19:36:26]

Done.

 
     const PlatformThreadId target;     // ID of The thread being sampled.
     const SamplingParams params;       // Information about how to sample.
     const CompletedCallback callback;  // Callback made when sampling complete.
     WaitableEvent* const finished;     // Signaled when all sampling complete.
 
     // Platform-specific module that does the actual sampling.
     std::unique_ptr<NativeStackSampler> native_sampler;
 
     // The absolute time for the next sample.
     Time next_sample_time;
 
     // The time that a profile was started, for calculating the total duration.
     Time profile_start_time;
 
     // Counters that indicate the current position along the acquisition.
     int burst = 0;
     int sample = 0;
 
     // The collected stack samples. The active profile is always at the back().
     CallStackProfiles profiles;
 
-   private:
     static StaticAtomicSequenceNumber next_collection_id_;

[Mike Wittman, 2017/07/06 17:25:53]

nit: drop trailing underscore

[Alexei Svitkine (slow), 2017/07/06 19:36:26]

Done.

   };
 
   // Gets the single instance of this class.
   static SamplingThread* GetInstance();
 
   // Adds a new CollectionContext to the thread. This can be called externally
   // from any thread. This returns an ID that can later be used to stop
   // the sampling.
   int Add(std::unique_ptr<CollectionContext> collection);
 
@@ skipping 29 matching lines @@
   ~SamplingThread() override;
 
   // Get task runner that is usable from the outside.
   scoped_refptr<SingleThreadTaskRunner> GetOrCreateTaskRunnerForAdd();
   scoped_refptr<SingleThreadTaskRunner> GetTaskRunner(
       ThreadExecutionState* out_state);
 
   // Get task runner that is usable from the sampling thread itself.
   scoped_refptr<SingleThreadTaskRunner> GetTaskRunnerOnSamplingThread();
 
-  // Finishes a collection and reports collected data via callback.
-  void FinishCollection(CollectionContext* collection);
+  // Finishes a collection and reports collected data via callback. Returns
+  // the new collection params, if a new collection should be started.
+  Optional<SamplingParams> FinishCollection(CollectionContext* collection);
 
   // Records a single sample of a collection.
   void RecordSample(CollectionContext* collection);
 
   // Check if the sampling thread is idle and begin a shutdown if it is.
   void ScheduleShutdownIfIdle();
 
   // These methods are tasks that get posted to the internal message queue.
   void AddCollectionTask(std::unique_ptr<CollectionContext> collection);
   void RemoveCollectionTask(int id);
@@ skipping 225 matching lines @@
 
 scoped_refptr<SingleThreadTaskRunner>
 StackSamplingProfiler::SamplingThread::GetTaskRunnerOnSamplingThread() {
   // This should be called only from the sampling thread as it has limited
   // accessibility.
   DCHECK_EQ(GetThreadId(), PlatformThread::CurrentId());
 
   return Thread::task_runner();
 }
 
-void StackSamplingProfiler::SamplingThread::FinishCollection(
+Optional<StackSamplingProfiler::SamplingParams>
+StackSamplingProfiler::SamplingThread::FinishCollection(
     CollectionContext* collection) {
   DCHECK_EQ(GetThreadId(), PlatformThread::CurrentId());
 
   // If there is no duration for the final profile (because it was stopped),
   // calculate it now.
   if (!collection->profiles.empty() &&
       collection->profiles.back().profile_duration == TimeDelta()) {
     collection->profiles.back().profile_duration =
         Time::Now() - collection->profile_start_time;
   }
 
   // Extract some information so callback and event-signalling can still be
   // done after the collection has been removed from the list of "active" ones.
   // This allows the the controlling object (and tests using it) to be confident
   // that collection is fully finished when those things occur.
   const CompletedCallback callback = collection->callback;
   CallStackProfiles profiles = std::move(collection->profiles);
   WaitableEvent* finished = collection->finished;
 
-  // Remove this collection from the map of known ones.  The |collection|
-  // parameter is invalid after this point.
-  size_t count = active_collections_.erase(collection->collection_id);
-  DCHECK_EQ(1U, count);
-
   // Run the associated callback, passing the collected profiles.
-  callback.Run(std::move(profiles));
+  Optional<SamplingParams> new_params = callback.Run(std::move(profiles));
 
   // Signal that this collection is finished.
   finished->Signal();
+
+  return new_params;
 }
 
 void StackSamplingProfiler::SamplingThread::RecordSample(
     CollectionContext* collection) {
   DCHECK_EQ(GetThreadId(), PlatformThread::CurrentId());
   DCHECK(collection->native_sampler);
 
   // If this is the first sample of a burst, a new Profile needs to be created
   // and filled.
   if (collection->sample == 0) {
@@ skipping 64 matching lines @@
   }
 }
 
 void StackSamplingProfiler::SamplingThread::RemoveCollectionTask(int id) {
   DCHECK_EQ(GetThreadId(), PlatformThread::CurrentId());
 
   auto found = active_collections_.find(id);
   if (found == active_collections_.end())
     return;
 
-  FinishCollection(found->second.get());
+  auto* collection = found->second.get();
+  FinishCollection(collection);
+
+  // Remove this collection from the map of known ones. After this,
+  // |collection| will be invalid.
+  size_t count = active_collections_.erase(collection->collection_id);
+  DCHECK_EQ(1U, count);
+
   ScheduleShutdownIfIdle();
 }
 
 void StackSamplingProfiler::SamplingThread::PerformCollectionTask(int id) {
   DCHECK_EQ(GetThreadId(), PlatformThread::CurrentId());
 
   auto found = active_collections_.find(id);
 
   // The task won't be found if it has been stopped.
   if (found == active_collections_.end())
     return;
 
   CollectionContext* collection = found->second.get();
 
   // Handle first-run with no "next time".
   if (collection->next_sample_time == Time())
     collection->next_sample_time = Time::Now();
 
   // Do the collection of a single sample.
   RecordSample(collection);
 
   // Update the time of the next sample recording.
-  if (UpdateNextSampleTime(collection)) {
-    bool success = GetTaskRunnerOnSamplingThread()->PostDelayedTask(
-        FROM_HERE,
-        BindOnce(&SamplingThread::PerformCollectionTask, Unretained(this), id),
-        std::max(collection->next_sample_time - Time::Now(), TimeDelta()));
-    DCHECK(success);
-  } else {
-    // All capturing has completed so finish the collection. By not re-adding
-    // it to the task queue, the collection will "expire" (i.e. no further work
-    // will be done). The |collection| variable will be invalid after this call.
-    FinishCollection(collection);
-    ScheduleShutdownIfIdle();
+  const bool collection_finished = !UpdateNextSampleTime(collection);
+  if (collection_finished) {
+    // All capturing has completed so finish the collection. If no new params
+    // are returned, a new collection should not be started.
+    Optional<SamplingParams> new_params = FinishCollection(collection);
+    if (!new_params.has_value()) {
+      // Remove this collection from the map of known ones. After this,
+      // |collection| will be invalid.
+      size_t count = active_collections_.erase(collection->collection_id);
+      DCHECK_EQ(1U, count);
+
+      // By not adding it to the task queue, the collection will "expire" (i.e.
+      // no further work will be done).
+      ScheduleShutdownIfIdle();
+      return;
+    }
+
+    // Restart the collection with the new params. Keep the same collection
+    // id so the Stop() operation continues to work.
+    auto new_collection = MakeUnique<SamplingThread::CollectionContext>(
+        collection->collection_id, collection->target, new_params.value(),
+        collection->callback, collection->finished,
+        std::move(collection->native_sampler));
+    new_collection->next_sample_time =

[Mike Wittman, 2017/07/06 17:25:52]

Can we reuse the logic in AddCollectionTask for restarting the collection
(exclusive of the add events increment)? This code is pretty complex, so the
fewer ways we have of doing something the better.

[Alexei Svitkine (slow), 2017/07/06 19:36:25]

The way it's structured right now, it re-uses the logic for scheduling the next
sample (i.e. line 651 is shared between schedule to take next sample normally or
when periodic starts). It's pretty straight forward because the two work
basically the same (via next_sample_time).

The functional difference when using AddCollectionTask() is that
|thread_execution_state_add_events_| will be incremented, but I don't think it
makes a difference in this case?

We can't use it outright because it uses active_collections_.insert() which
won't replace an element. And if we made it replace it (e.g. by using
active_collections_[profiler_id] = syntax), it will hide away the invalidation
of |collection|. It will also be a bit less efficient since we'd need to do the
map lookup again.

So my preference is to keep it as-is so that the code is shared with the "next
sample case" here rather than "starting collection task". Another option is to
make a helper function wrapping the PostDelayedTask that both places could use.

[Mike Wittman, 2017/07/06 22:14:01]

I was thinking that we could remove the old collection then call
AddCollectionTask() on the new collection. I think that will result in
duplicating code to erase the old collection though (vs. the current
implementation which has duplicated code to handle the initial delay). Neither
is obviously better, so sticking with the current implementation sgtm.

+        Time::Now() + collection->params.initial_delay;
+    // Replace |collection| in the map by |new_collection|. After this,
+    // |collection| will be invalid.
+    found->second = std::move(new_collection);
   }
+
+  bool success = GetTaskRunnerOnSamplingThread()->PostDelayedTask(
+      FROM_HERE,
+      BindOnce(&SamplingThread::PerformCollectionTask, Unretained(this), id),
+      std::max(collection->next_sample_time - Time::Now(), TimeDelta()));
+  DCHECK(success);
 }
 
 void StackSamplingProfiler::SamplingThread::ShutdownTask(int add_events) {
   DCHECK_EQ(GetThreadId(), PlatformThread::CurrentId());
 
   // Holding this lock ensures that any attempt to start another job will
   // get postponed until |thread_execution_state_| is updated, thus eliminating
   // the race in starting a new thread while the previous one is exiting.
   AutoLock lock(thread_execution_state_lock_);
 
@@ skipping 139 matching lines @@
   if (!native_sampler)
     return;
 
   // Wait for profiling to be "inactive", then reset it for the upcoming run.
   profiling_inactive_.Wait();
   profiling_inactive_.Reset();
 
   DCHECK_EQ(NULL_COLLECTION_ID, collection_id_);
   collection_id_ = SamplingThread::GetInstance()->Add(
       MakeUnique<SamplingThread::CollectionContext>(
+          SamplingThread::CollectionContext::next_collection_id_.GetNext(),
           thread_id_, params_, completed_callback_, &profiling_inactive_,
           std::move(native_sampler)));
   DCHECK_NE(NULL_COLLECTION_ID, collection_id_);
 }
 
 void StackSamplingProfiler::Stop() {
   SamplingThread::GetInstance()->Remove(collection_id_);
   collection_id_ = NULL_COLLECTION_ID;
 }
 
@@ skipping 48 matching lines @@
 }
 
 bool operator<(const StackSamplingProfiler::Frame &a,
                const StackSamplingProfiler::Frame &b) {
   return (a.module_index < b.module_index) ||
       (a.module_index == b.module_index &&
        a.instruction_pointer < b.instruction_pointer);
 }
 
 }  // namespace base