Support parallel captures from the StackSamplingProfiler.

base/profiler/stack_sampling_profiler_unittest.cc

Index: base/profiler/stack_sampling_profiler_unittest.cc
diff --git a/base/profiler/stack_sampling_profiler_unittest.cc b/base/profiler/stack_sampling_profiler_unittest.cc
index 16205ac2b09f22e25975cda252c8306ba6eb918a..d59686f6f1b74d61a1199fba9482a678ca21923a 100644
--- a/base/profiler/stack_sampling_profiler_unittest.cc
+++ b/base/profiler/stack_sampling_profiler_unittest.cc
@@ -444,9 +444,9 @@ void TestLibraryUnload(bool wait_until_unloaded) {
     StackCopiedSignaler(WaitableEvent* stack_copied,
                         WaitableEvent* start_stack_walk,
                         bool wait_to_walk_stack)
-        : stack_copied_(stack_copied), start_stack_walk_(start_stack_walk),
-          wait_to_walk_stack_(wait_to_walk_stack) {
-    }
+        : stack_copied_(stack_copied),
+          start_stack_walk_(start_stack_walk),
+          wait_to_walk_stack_(wait_to_walk_stack) {}
 
     void OnPreStackWalk() override {
       stack_copied_->Signal();
@@ -720,35 +720,6 @@ TEST(StackSamplingProfilerTest, MAYBE_Alloca) {
       << FormatSampleForDiagnosticOutput(sample, profile.modules);
 }
 
-// Checks that the fire-and-forget interface works.
-#if defined(STACK_SAMPLING_PROFILER_SUPPORTED)
-#define MAYBE_StartAndRunAsync StartAndRunAsync
-#else
-#define MAYBE_StartAndRunAsync DISABLED_StartAndRunAsync
-#endif
-TEST(StackSamplingProfilerTest, MAYBE_StartAndRunAsync) {
-  // StartAndRunAsync requires the caller to have a message loop.
-  MessageLoop message_loop;
-
-  SamplingParams params;
-  params.samples_per_burst = 1;
-
-  CallStackProfiles profiles;
-  WithTargetThread([&params, &profiles](PlatformThreadId target_thread_id) {
-    WaitableEvent sampling_thread_completed(
-        WaitableEvent::ResetPolicy::AUTOMATIC,
-        WaitableEvent::InitialState::NOT_SIGNALED);
-    const StackSamplingProfiler::CompletedCallback callback =
-        Bind(&SaveProfilesAndSignalEvent, Unretained(&profiles),
-             Unretained(&sampling_thread_completed));
-    StackSamplingProfiler::StartAndRunAsync(target_thread_id, params, callback);
-    RunLoop().RunUntilIdle();
-    sampling_thread_completed.Wait();
-  });
-
-  ASSERT_EQ(1u, profiles.size());
-}
-
 // Checks that the expected number of profiles and samples are present in the
 // call stack profiles produced.
 #if defined(STACK_SAMPLING_PROFILER_SUPPORTED)
@@ -860,6 +831,46 @@ TEST(StackSamplingProfilerTest, MAYBE_DestroyProfilerWhileProfiling) {
 #define MAYBE_CanRunMultipleTimes DISABLED_CanRunMultipleTimes
 #endif
 TEST(StackSamplingProfilerTest, MAYBE_CanRunMultipleTimes) {
+  StackSamplingProfiler::TestAPI::SetSamplingThreadIdleShutdownTime(0);
+
+  WithTargetThread([](PlatformThreadId target_thread_id) {
+    SamplingParams params;
+    params.sampling_interval = TimeDelta::FromMilliseconds(0);
+    params.samples_per_burst = 1;
+
+    CallStackProfiles profiles;
+    WaitableEvent sampling_completed(WaitableEvent::ResetPolicy::MANUAL,
+                                     WaitableEvent::InitialState::NOT_SIGNALED);
+    const StackSamplingProfiler::CompletedCallback callback =
+        Bind(&SaveProfilesAndSignalEvent, Unretained(&profiles),
+             Unretained(&sampling_completed));
+    StackSamplingProfiler profiler(target_thread_id, params, callback);
+
+    // Just start and stop to execute code paths.
+    profiler.Start();
+    profiler.Stop();
+    sampling_completed.Wait();
+
+    // Ensure a second request will run and not block.
+    sampling_completed.Reset();
+    profiles.clear();
+    profiler.Start();
+    sampling_completed.TimedWait(AVeryLongTimeDelta());

[Mike Wittman, 2017/02/22 20:32:18]

What's the reason for the TimedWait?

[bcwhite, 2017/02/24 20:39:24]

To ensure that it runs to completion before being stopped.  Otherwise it could
stop before the first sample and there wouldn't be any evidence that it would
have run.

[Mike Wittman, 2017/02/27 23:27:34]

Right, but why not just Wait(), and remove the Wait() call below?

[bcwhite, 2017/03/13 18:50:17]

Done.

+    profiler.Stop();
+    sampling_completed.Wait();
+    ASSERT_EQ(1u, profiles.size());
+  });
+}
+
+// Checks that the different profilers may be run.
+#if defined(STACK_SAMPLING_PROFILER_SUPPORTED)
+#define MAYBE_CanRunMultipleProfilers CanRunMultipleProfilers
+#else
+#define MAYBE_CanRunMultipleProfilers DISABLED_CanRunMultipleProfilers
+#endif
+TEST(StackSamplingProfilerTest, MAYBE_CanRunMultipleProfilers) {
+  StackSamplingProfiler::TestAPI::SetSamplingThreadIdleShutdownTime(0);
+
   SamplingParams params;
   params.sampling_interval = TimeDelta::FromMilliseconds(0);
   params.samples_per_burst = 1;
@@ -873,22 +884,55 @@ TEST(StackSamplingProfilerTest, MAYBE_CanRunMultipleTimes) {
   ASSERT_EQ(1u, profiles.size());
 }
 
-// Checks that requests to start profiling while another profile is taking place
-// are ignored.
+// Checks that additional requests will restart a stopped profiler.
 #if defined(STACK_SAMPLING_PROFILER_SUPPORTED)
-#define MAYBE_ConcurrentProfiling ConcurrentProfiling
+#define MAYBE_WillRestartSampler WillRestartSampler
 #else
-#define MAYBE_ConcurrentProfiling DISABLED_ConcurrentProfiling
+#define MAYBE_WillRestartSampler DISABLED_WillRestartSampler
 #endif
-TEST(StackSamplingProfilerTest, MAYBE_ConcurrentProfiling) {
+TEST(StackSamplingProfilerTest, MAYBE_WillRestartSampler) {
+  StackSamplingProfiler::TestAPI::SetSamplingThreadIdleShutdownTime(0);
+
+  SamplingParams params;
+  params.sampling_interval = TimeDelta::FromMilliseconds(0);
+  params.samples_per_burst = 1;
+
+  std::vector<CallStackProfile> profiles;
+  CaptureProfiles(params, AVeryLongTimeDelta(), &profiles);
+  ASSERT_EQ(1u, profiles.size());
+
+  // Capture thread should still be running at this point.
+  ASSERT_TRUE(StackSamplingProfiler::TestAPI::IsSamplingThreadRunning());
+
+  // Initiate an "idle" shutdown.  The task will be run immediately but on
+  // another thread so wait for it to complete.
+  StackSamplingProfiler::TestAPI::InitiateSamplingThreadIdleShutdown();
+  while (StackSamplingProfiler::TestAPI::IsSamplingThreadRunning())
+    PlatformThread::YieldCurrentThread();

[Mike Wittman, 2017/02/22 20:32:18]

Channeling brucedawson@: while (condition) yield(); results in a busy wait if
there spare execution cycles in the system, where the thread is repeatedly
scheduled, executes, and yields. This is bad for power usage. Admittedly this is
not a big concern in tests, but people do tend to copy-paste code around. The
preferred formulation is while (condition) sleep(1); to allow the processor to
be idle for some time.

[bcwhite, 2017/03/13 18:50:17]

Done.

[Mike Wittman, 2017/03/14 18:57:33]

Sleeping for 1ms is preferable, to minimize test execution time.

[bcwhite, 2017/03/16 15:56:24]

Done.

+
+  // Ensure another capture will start the sampling thread and run.
+  profiles.clear();
+  CaptureProfiles(params, AVeryLongTimeDelta(), &profiles);
+  ASSERT_EQ(1u, profiles.size());
+  EXPECT_TRUE(StackSamplingProfiler::TestAPI::IsSamplingThreadRunning());
+}
+
+// Checks that multiple sampling requests execute in parallel (#1).
+#if defined(STACK_SAMPLING_PROFILER_SUPPORTED)
+#define MAYBE_ConcurrentProfiling1 ConcurrentProfiling1

[Mike Wittman, 2017/02/22 20:32:18]

Test name and comment should be descriptive of the specific conditions this is
testing, rather than using a numeric suffix.

[bcwhite, 2017/02/24 20:39:24]

Done.

+#else
+#define MAYBE_ConcurrentProfiling1 DISABLED_ConcurrentProfiling1
+#endif
+TEST(StackSamplingProfilerTest, MAYBE_ConcurrentProfiling1) {
   WithTargetThread([](PlatformThreadId target_thread_id) {
     SamplingParams params[2];
     params[0].initial_delay = TimeDelta::FromMilliseconds(10);
-    params[0].sampling_interval = TimeDelta::FromMilliseconds(0);
-    params[0].samples_per_burst = 1;
+    params[0].sampling_interval = TimeDelta::FromMilliseconds(1);
+    params[0].samples_per_burst = 10;
 
-    params[1].sampling_interval = TimeDelta::FromMilliseconds(0);
-    params[1].samples_per_burst = 1;
+    params[1].initial_delay = TimeDelta::FromMilliseconds(10);
+    params[1].sampling_interval = TimeDelta::FromMilliseconds(1);
+    params[1].samples_per_burst = 10;
 
     CallStackProfiles profiles[2];
     std::vector<std::unique_ptr<WaitableEvent>> sampling_completed(2);
@@ -919,14 +963,115 @@ TEST(StackSamplingProfilerTest, MAYBE_ConcurrentProfiling) {
     EXPECT_EQ(1u, profiles[completed_profiler].size());
 
     size_t other_profiler = 1 - completed_profiler;
-    // Give the other profiler a chance to run and observe that it hasn't.
-    EXPECT_FALSE(sampling_completed[other_profiler]->TimedWait(
-        TimeDelta::FromMilliseconds(25)));
-
-    // Start the other profiler again and it should run.
-    profiler[other_profiler]->Start();
-    sampling_completed[other_profiler]->Wait();
-    EXPECT_EQ(1u, profiles[other_profiler].size());
+    // Give the other profiler a chance to finish and verify it does so.
+    EXPECT_TRUE(sampling_completed[other_profiler]->TimedWait(
+        TimeDelta::FromMilliseconds(250)));
+  });
+}
+
+// Checks that multiple sampling requests execute in parallel (#2).
+#if defined(STACK_SAMPLING_PROFILER_SUPPORTED)
+#define MAYBE_ConcurrentProfiling2 ConcurrentProfiling2
+#else
+#define MAYBE_ConcurrentProfiling2 DISABLED_ConcurrentProfiling2
+#endif
+TEST(StackSamplingProfilerTest, MAYBE_ConcurrentProfiling2) {
+  WithTargetThread([](PlatformThreadId target_thread_id) {
+    SamplingParams params[2];
+    params[0].initial_delay = TimeDelta::FromMilliseconds(10);
+    params[0].sampling_interval = TimeDelta::FromMilliseconds(2);
+    params[0].samples_per_burst = 9;
+
+    params[1].initial_delay = TimeDelta::FromMilliseconds(10);
+    params[1].sampling_interval = TimeDelta::FromMilliseconds(3);
+    params[1].samples_per_burst = 6;
+
+    CallStackProfiles profiles[2];
+    std::vector<std::unique_ptr<WaitableEvent>> sampling_completed(2);
+    std::vector<std::unique_ptr<StackSamplingProfiler>> profiler(2);
+    for (int i = 0; i < 2; ++i) {
+      sampling_completed[i] =
+          MakeUnique<WaitableEvent>(WaitableEvent::ResetPolicy::AUTOMATIC,
+                                    WaitableEvent::InitialState::NOT_SIGNALED);
+      const StackSamplingProfiler::CompletedCallback callback =
+          Bind(&SaveProfilesAndSignalEvent, Unretained(&profiles[i]),
+               Unretained(sampling_completed[i].get()));
+      profiler[i] = MakeUnique<StackSamplingProfiler>(target_thread_id,
+                                                      params[i], callback);
+    }
+
+    for (size_t i = 0; i < profiler.size(); ++i)
+      profiler[i]->Start();
+
+    std::vector<WaitableEvent*> sampling_completed_rawptrs(
+        sampling_completed.size());
+    std::transform(
+        sampling_completed.begin(), sampling_completed.end(),
+        sampling_completed_rawptrs.begin(),
+        [](const std::unique_ptr<WaitableEvent>& elem) { return elem.get(); });
+    // Wait for one profiler to finish.
+    size_t completed_profiler =
+        WaitableEvent::WaitMany(sampling_completed_rawptrs.data(), 2);
+    EXPECT_EQ(1u, profiles[completed_profiler].size());
+    // Stop and destroy all the profilers. Don't crash.
+    for (size_t i = 0; i < profiler.size(); ++i)
+      profiler[i]->Stop();
+    for (size_t i = 0; i < profiler.size(); ++i)
+      profiler[i].reset();
+  });
+}
+
+// Checks that multiple sampling requests execute in parallel (#3).
+#if defined(STACK_SAMPLING_PROFILER_SUPPORTED)
+#define MAYBE_ConcurrentProfiling3 ConcurrentProfiling3
+#else
+#define MAYBE_ConcurrentProfiling3 DISABLED_ConcurrentProfiling3
+#endif
+TEST(StackSamplingProfilerTest, MAYBE_ConcurrentProfiling3) {
+  WithTargetThread([](PlatformThreadId target_thread_id) {
+    SamplingParams params[3];
+    params[0].initial_delay = TimeDelta::FromMilliseconds(8);
+    params[0].sampling_interval = TimeDelta::FromMilliseconds(4);
+    params[0].samples_per_burst = 10;
+
+    params[1].initial_delay = TimeDelta::FromMilliseconds(9);
+    params[1].sampling_interval = TimeDelta::FromMilliseconds(3);
+    params[1].samples_per_burst = 10;
+
+    params[2].initial_delay = TimeDelta::FromMilliseconds(10);
+    params[2].sampling_interval = TimeDelta::FromMilliseconds(2);
+    params[2].samples_per_burst = 10;
+
+    CallStackProfiles profiles[3];
+    std::vector<std::unique_ptr<WaitableEvent>> sampling_completed(3);
+    std::vector<std::unique_ptr<StackSamplingProfiler>> profiler(3);
+    for (int i = 0; i < 3; ++i) {
+      sampling_completed[i] =
+          MakeUnique<WaitableEvent>(WaitableEvent::ResetPolicy::AUTOMATIC,
+                                    WaitableEvent::InitialState::NOT_SIGNALED);
+      const StackSamplingProfiler::CompletedCallback callback =
+          Bind(&SaveProfilesAndSignalEvent, Unretained(&profiles[i]),
+               Unretained(sampling_completed[i].get()));
+      profiler[i] = MakeUnique<StackSamplingProfiler>(target_thread_id,
+                                                      params[i], callback);
+    }
+
+    for (size_t i = 0; i < profiler.size(); ++i)
+      profiler[i]->Start();
+
+    std::vector<WaitableEvent*> sampling_completed_rawptrs(
+        sampling_completed.size());
+    std::transform(
+        sampling_completed.begin(), sampling_completed.end(),
+        sampling_completed_rawptrs.begin(),
+        [](const std::unique_ptr<WaitableEvent>& elem) { return elem.get(); });
+    // Wait for one profiler to finish.
+    size_t completed_profiler =
+        WaitableEvent::WaitMany(sampling_completed_rawptrs.data(), 2);

[Mike Wittman, 2017/02/22 20:32:18]

2 => 3, or better, arraysize(params). Since we're duplicating and changing tests
here, we probably should replace all the constants here and in the previous
tests with arraysize(params) to make this less fragile.

[bcwhite, 2017/02/24 20:39:24]

Done.

+    EXPECT_EQ(1u, profiles[completed_profiler].size());
+    // Destroy all the profilers. Don't crash.
+    for (size_t i = 0; i < profiler.size(); ++i)
+      profiler[i].reset();
   });
 }
 

[Mike Wittman, 2017/02/22 20:32:18]

I'd like to have a test that interleaves Start and Stop calls on different
profilers as well.

[bcwhite, 2017/02/24 20:39:24]

Done.