Support parallel captures from the StackSamplingProfiler.

base/profiler/stack_sampling_profiler.h

 // 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.
 
 #ifndef BASE_PROFILER_STACK_SAMPLING_PROFILER_H_
 #define BASE_PROFILER_STACK_SAMPLING_PROFILER_H_
 
 #include <stddef.h>
 
 #include <memory>
@@ skipping 161 matching lines @@
     TimeDelta burst_interval = TimeDelta::FromSeconds(10);
 
     // Number of samples to record per burst.
     int samples_per_burst = 300;
 
     // Interval between samples during a sampling burst. This is the desired
     // duration from the start of one sample to the start of the next sample.
     TimeDelta sampling_interval = TimeDelta::FromMilliseconds(100);
   };
 
+  // Testing support. These methods are static beause they interact with the
+  // sampling thread, a singleton used by all StackSamplingProfiler objects.
+  class BASE_EXPORT TestAPI {
+   public:
+    // Resets the internal state to that of a fresh start. This is necessary
+    // so that tests don't inherit state from previous tests.
+    static void Reset();
+
+    // Resets internal annotations (like process phase) to initial values.
+    static void ResetAnnotations();
+
+    // Returns whether the sampling thread is currently running or not.
+    static bool IsSamplingThreadRunning();

[gab, 2017/04/05 20:38:43]

Thread::IsRunning() isn't thread-safe (though the check is sadly disabled right
now [1]) and as such this method isn't either (must be called from owning
sequence). Please document it as such (or probably this entire TestAPI class as
such in fact).

[1]
https://cs.chromium.org/chromium/src/base/threading/thread.cc?type=cs&q=%22//...

[bcwhite, 2017/04/06 18:40:18]

I see.  Would the best solution be to have CleanUp set a flag (under lock) and
return that?
Done.  I also added a DetachFromSequence in the code for this.  Let me know if
that's unnecessary.

[gab, 2017/04/06 19:31:28]

Hmmm if the comment above is respected and IsRunning() is always called from
owning thread then it's always fine. No need to have a fancy Cleanup() -- that's
already what Thread::IsRunning() is doing when called properly.

+
+    // Disables inherent idle-shutdown behavior.
+    static void DisableIdleShutdown();

[gab, 2017/04/05 20:38:42]

Since you should support having multiple pending delayed shutdown tasks in your
queue (I don't see anything that prevents that from happening), why bother
disable them? Your tests should complete much before they fire for real anyways
so it shouldn't be a source of flakiness, disabling them merely brings you
further from testing your real product code.

[bcwhite, 2017/04/06 18:40:18]

They can and it's handled.
Just for guaranteed operation.  Right now the idle shutdown time is 1 minute but
it's an internal thing.  If it was changed to 1s we wouldn't want the tests to
become flaky.

[gab, 2017/04/06 19:31:28]

Hmm okay, but you also by doing so don't test calling ShutdownTask with other
pending ShutdownTasks.

Up to you and wittman@ to decide which you prefer, just highlighting this.

[Mike Wittman, 2017/04/06 21:54:30]

Yes, the tests don't explicitly generate multiple ShutdownTasks, but we
extensively considered this behavior in the review and I'm satisfied that all
the code paths exercised in this case are adequately tested.

+
+    // Initiates an idle shutdown task, as though the idle timer had expired,
+    // causing the thread to exit. There is no "idle" check so this must be
+    // called only when all sampling tasks have completed. This blocks until
+    // the task has been executed, though the actual stopping of the thread
+    // still happens asynchronously. Watch IsSamplingThreadRunning() to know
+    // when the thread has exited. If |simulate_intervening_start| is true then
+    // this method will make it appear to the shutdown task that a new profiler
+    // was started between when the idle-shutdown was initiated and when it
+    // runs.
+    static void PerformSamplingThreadIdleShutdown(
+        bool simulate_intervening_start);
+  };
+
   // The callback type used to collect completed profiles. The passed |profiles|
-  // are move-only.
+  // are move-only. Other threads, including the UI thread, may block on
+  // callback completion so this should run as quickly as possible.
   //
-  // IMPORTANT NOTE: the callback is invoked on a thread the profiler
+  // IMPORTANT NOTE: The callback is invoked on a thread the profiler
   // constructs, rather than on the thread used to construct the profiler and
   // set the callback, and thus the callback must be callable on any thread. For
   // threads with message loops that create StackSamplingProfilers, posting a
-  // task to the message loop with a copy of the profiles is the recommended
+  // task to the message loop with the moved (i.e. std::move) profiles is the
   // thread-safe callback implementation.
   using CompletedCallback = Callback<void(CallStackProfiles)>;
 
-  // Creates a profiler that sends completed profiles to |callback|. The second
-  // constructor is for test purposes.
-  StackSamplingProfiler(PlatformThreadId thread_id,
-                        const SamplingParams& params,
-                        const CompletedCallback& callback);
-  StackSamplingProfiler(PlatformThreadId thread_id,
-                        const SamplingParams& params,
-                        const CompletedCallback& callback,
-                        NativeStackSamplerTestDelegate* test_delegate);
+  // Creates a profiler for the CURRENT thread that sends completed profiles
+  // to |callback|. An optional |test_delegate| can be supplied by tests.
+  // The caller must ensure that this object gets destroyed before the current
+  // thread exits.
+  StackSamplingProfiler(
+      const SamplingParams& params,
+      const CompletedCallback& callback,
+      NativeStackSamplerTestDelegate* test_delegate = nullptr);
+
+  // Creates a profiler for ANOTHER thread that sends completed profiles to
+  // |callback|. An optional |test_delegate| can be supplied by tests.
+  //
+  // IMPORTANT: The caller must ensure that the thread being sampled does not
+  // exit before this object gets destructed or Bad Things(tm) may occur.
+  StackSamplingProfiler(
+      PlatformThreadId thread_id,
+      const SamplingParams& params,
+      const CompletedCallback& callback,
+      NativeStackSamplerTestDelegate* test_delegate = nullptr);
+
   // Stops any profiling currently taking place before destroying the profiler.
+  // This will block until the callback has been run if profiling has started
+  // but not already finished.
   ~StackSamplingProfiler();
 
-  // The fire-and-forget interface: starts a profiler and allows it to complete
-  // without the caller needing to manage the profiler lifetime. May be invoked
-  // from any thread, but requires that the calling thread has a message loop.
-  static void StartAndRunAsync(PlatformThreadId thread_id,
-                               const SamplingParams& params,
-                               const CompletedCallback& callback);
-
   // Initializes the profiler and starts sampling.
   void Start();
 
-  // Stops the profiler and any ongoing sampling. Calling this function is
-  // optional; if not invoked profiling terminates when all the profiling bursts
-  // specified in the SamplingParams are completed or the profiler is destroyed,
-  // whichever occurs first.
+  // Stops the profiler and any ongoing sampling. This method will return
+  // immediately with the callback being run asynchronously. At most one
+  // more stack sample will be taken after this method returns. Calling this
+  // function is optional; if not invoked profiling terminates when all the
+  // profiling bursts specified in the SamplingParams are completed or the
+  // profiler object is destroyed, whichever occurs first.
   void Stop();
 
   // Set the current system state that is recorded with each captured stack
   // frame. This is thread-safe so can be called from anywhere. The parameter
   // value should be from an enumeration of the appropriate type with values
   // ranging from 0 to 31, inclusive. This sets bits within Sample field of
   // |process_milestones|. The actual meanings of these bits are defined
   // (globally) by the caller(s).
   static void SetProcessMilestone(int milestone);
-  static void ResetAnnotationsForTesting();
 
  private:
+  friend class TestAPI;
+
   // SamplingThread is a separate thread used to suspend and sample stacks from
   // the target thread.
-  class SamplingThread : public PlatformThread::Delegate {
-   public:
-    // Samples stacks using |native_sampler|. When complete, invokes
-    // |completed_callback| with the collected call stack profiles.
-    // |completed_callback| must be callable on any thread.
-    SamplingThread(std::unique_ptr<NativeStackSampler> native_sampler,
-                   const SamplingParams& params,
-                   const CompletedCallback& completed_callback);
-    ~SamplingThread() override;
-
-    // PlatformThread::Delegate:
-    void ThreadMain() override;
-
-    void Stop();
-
-   private:
-    // Collects |profile| from a single burst. If the profiler was stopped
-    // during collection, sets |was_stopped| and provides the set of samples
-    // collected up to that point.
-    void CollectProfile(CallStackProfile* profile, TimeDelta* elapsed_time,
-                        bool* was_stopped);
-
-    // Collects call stack profiles from all bursts, or until the sampling is
-    // stopped. If stopped before complete, the last profile in
-    // |call_stack_profiles| may contain a partial burst.
-    void CollectProfiles(CallStackProfiles* profiles);
-
-    std::unique_ptr<NativeStackSampler> native_sampler_;
-    const SamplingParams params_;
-
-    // If Stop() is called, it signals this event to force the sampling to
-    // terminate before all the samples specified in |params_| are collected.
-    WaitableEvent stop_event_;
-
-    const CompletedCallback completed_callback_;
-
-    DISALLOW_COPY_AND_ASSIGN(SamplingThread);
-  };
+  class SamplingThread;
 
   // Adds annotations to a Sample.
   static void RecordAnnotations(Sample* sample);
 
   // This global variables holds the current system state and is recorded with
   // every captured sample, done on a separate thread which is why updates to
   // this must be atomic. A PostTask to move the the updates to that thread
   // would skew the timing and a lock could result in deadlock if the thread
   // making a change was also being profiled and got stopped.
   static subtle::Atomic32 process_milestones_;
 
   // The thread whose stack will be sampled.
   PlatformThreadId thread_id_;
 
   const SamplingParams params_;
 
-  std::unique_ptr<SamplingThread> sampling_thread_;
-  PlatformThreadHandle sampling_thread_handle_;
+  const CompletedCallback completed_callback_;
 
-  const CompletedCallback completed_callback_;
+  // This starts "signaled", is reset when sampling begins, and is signaled
+  // when that sampling is complete and the callback done.
+  WaitableEvent profiling_inactive_;
+
+  // Object that does the native sampling. This is created during construction
+  // and later passed to the sampling thread when profiling is started.
+  std::unique_ptr<NativeStackSampler> native_sampler_;
+
+  // An ID uniquely identifying this collection to the sampling thread. This
+  // will be an internal "null" value when no collection has been started.
+  int collection_id_;
 
   // Stored until it can be passed to the NativeStackSampler created in Start().
   NativeStackSamplerTestDelegate* const test_delegate_;
 
   DISALLOW_COPY_AND_ASSIGN(StackSamplingProfiler);
 };
 
 // These operators permit types to be compared and used in a map of Samples, as
 // done in tests and by the metrics provider code.
 BASE_EXPORT bool operator==(const StackSamplingProfiler::Module& a,
                             const StackSamplingProfiler::Module& b);
 BASE_EXPORT bool operator==(const StackSamplingProfiler::Sample& a,
                             const StackSamplingProfiler::Sample& b);
 BASE_EXPORT bool operator!=(const StackSamplingProfiler::Sample& a,
                             const StackSamplingProfiler::Sample& b);
 BASE_EXPORT bool operator<(const StackSamplingProfiler::Sample& a,
                            const StackSamplingProfiler::Sample& b);
 BASE_EXPORT bool operator==(const StackSamplingProfiler::Frame& a,
                             const StackSamplingProfiler::Frame& b);
 BASE_EXPORT bool operator<(const StackSamplingProfiler::Frame& a,
                            const StackSamplingProfiler::Frame& b);
 
 }  // namespace base
 
 #endif  // BASE_PROFILER_STACK_SAMPLING_PROFILER_H_