Support parallel captures from the StackSamplingProfiler.

base/profiler/stack_sampling_profiler_unittest.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 <stddef.h>
 #include <stdint.h>
 
 #include <cstdlib>
 #include <memory>
 #include <utility>
@@ skipping 356 matching lines @@
         Bind(&SaveProfilesAndSignalEvent, Unretained(profiles),
              Unretained(&sampling_thread_completed));
     StackSamplingProfiler profiler(target_thread_id, params, callback);
     profiler.Start();
     sampling_thread_completed.TimedWait(profiler_wait_time);
     profiler.Stop();
     sampling_thread_completed.Wait();
   });
 }
 
+// Waits for one of multiple samplings to complete.
+size_t WaitForSamplingComplete(
+    std::vector<std::unique_ptr<WaitableEvent>>* sampling_completed) {
+  // Map unique_ptrs to something that WaitMany can accept.
+  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.
+  return WaitableEvent::WaitMany(sampling_completed_rawptrs.data(),
+                                 sampling_completed_rawptrs.size());
+}
+
 // If this executable was linked with /INCREMENTAL (the default for non-official
 // debug and release builds on Windows), function addresses do not correspond to
 // function code itself, but instead to instructions in the Incremental Link
 // Table that jump to the functions. Checks for a jump instruction and if
 // present does a little decompilation to find the function's actual starting
 // address.
 const void* MaybeFixupFunctionAddressForILT(const void* function_address) {
 #if defined(_WIN64)
   const unsigned char* opcode =
       reinterpret_cast<const unsigned char*>(function_address);
@@ skipping 50 matching lines @@
 // asynchronous library loading has completed before walking the stack. If
 // false, the unloading may still be occurring during the stack walk.
 void TestLibraryUnload(bool wait_until_unloaded) {
   // Test delegate that supports intervening between the copying of the stack
   // and the walking of the stack.
   class StackCopiedSignaler : public NativeStackSamplerTestDelegate {
    public:
     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();
       if (wait_to_walk_stack_)
         start_stack_walk_->Wait();
     }
 
    private:
     WaitableEvent* const stack_copied_;
     WaitableEvent* const start_stack_walk_;
@@ skipping 253 matching lines @@
              reinterpret_cast<const void*>(&TargetThread::CallWithAlloca))
       << " was not found in stack:\n"
       << FormatSampleForDiagnosticOutput(sample, profile.modules);
 
   // These frames should be adjacent on the stack.
   EXPECT_EQ(1, alloca_frame - end_frame)
       << "Stack:\n"
       << 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)
 #define MAYBE_MultipleProfilesAndSamples MultipleProfilesAndSamples
 #else
 #define MAYBE_MultipleProfilesAndSamples DISABLED_MultipleProfilesAndSamples
 #endif
 TEST(StackSamplingProfilerTest, MAYBE_MultipleProfilesAndSamples) {
   SamplingParams params;
   params.burst_interval = params.sampling_interval =
@@ skipping 91 matching lines @@
   });
 }
 
 // Checks that the same profiler may be run multiple times.
 #if defined(STACK_SAMPLING_PROFILER_SUPPORTED)
 #define MAYBE_CanRunMultipleTimes CanRunMultipleTimes
 #else
 #define MAYBE_CanRunMultipleTimes DISABLED_CanRunMultipleTimes
 #endif
 TEST(StackSamplingProfilerTest, MAYBE_CanRunMultipleTimes) {
+  StackSamplingProfiler::TestAPI::DisableIdleShutdown();
+
+  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.Wait();
+    profiler.Stop();
+    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::DisableIdleShutdown();
+
   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());
 
   profiles.clear();
   CaptureProfiles(params, AVeryLongTimeDelta(), &profiles);
   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::DisableIdleShutdown();
+
+  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::Sleep(base::TimeDelta::FromMilliseconds(100));
+
+  // 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 synchronized multiple sampling requests execute in parallel.
+#if defined(STACK_SAMPLING_PROFILER_SUPPORTED)
+#define MAYBE_ConcurrentProfiling_InSync ConcurrentProfiling_InSync
+#else
+#define MAYBE_ConcurrentProfiling_InSync DISABLED_ConcurrentProfiling_InSync
+#endif
+TEST(StackSamplingProfilerTest, MAYBE_ConcurrentProfiling_InSync) {

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

General comments on testing this functionality, now that I mostly understand
what the current tests are doing:

1. The most important aspects of this change to test are the subtleties around
collection and thread lifetime. We should have dedicated tests exercising the
different conditional outcomes for all the conditionals in
GetOrCreateTaskRunnerForAdd, Remove, FinishCollection, and ShutdownTask, and for
the id-not-found cases in RemoveCollectionTask and PerformCollectionTask.

There are several interleavings of Start/Stop/destroy in the current tests, but
it's not clear which of the cases are being exercised by them, which is why they
should have dedicated tests.

2. Testing that multiple profilers can run concurrently is important, but it's
not clear to me what the value is in testing multiple sampling interleavings.
The sampling interleaving is mostly an implementation detail of the profiler --
users won't care how they're interleaved with other collections as long as their
samples are collected close to "on time". The interleaving behavior is also
substantially provided by the message loop, so tests of different interleavings
are testing mostly that behavior not the behavior implemented in this class.
Perhaps I'm missing something though -- is there another reason to test multiple
interleavings?

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

GetOrCreateTaskRunnerForAdd:
- state==RUNNING: tested by ConcurrentProfiling_*
  Entered when doing parallel sampling.
- state==EXITING: tested by WillRestartSampler
  Entered after sampler shutdown.
- state==other: tested by everything
  Every first sampler will exercise this state.

Remove:
- state!=RUNNING: added StopAfterIdle test
- state==other: tested by every valid Stop()

ShutdownTask:
- not idle: tested everywhere
- changed add-event
  This is to handle a race condition which is necessarily difficult
  (if not impossible) to test.

RemoveCollectionTask:
- found: tested every stop before completion
- not-found: tested every stop after completion

PerformCollectionTask:
- found: tested with every valid sample
- not-found: tested every stop before completion
Removed one of the tests.

[Mike Wittman, 2017/03/18 01:38:41]

Relying on existing tests is OK if the test is obviously and reliably testing
the particular behavior. All the other cases need dedicated tests so that (1)
it's clear that the behavior is important and that it's actually being tested,
and (2) the test of the behavior survives changes and refactoring to the code
and tests. In particular, the behavior under test in several of the cases below
is subject to races dependent on the sampling parameters, system load, and
shutdown idle time.
We can't rely on the RUNNING state being tested in the ConcurrentProfiling_*
tests because the state in the second and later Start() calls is subject to
races dependent on the sampling parameters, the load on the system, and the
shutdown idle time.
This is a good test for this behavior.
This is also a good test for this behavior.
We can't rely on state==other(EXITING) being tested in Remove for the same
configuration/load-dependent reasons as for state==RUNNING in
GetOrCreateTaskRunnerForAdd.
This is not tested everywhere, If I'm not mistaken, since ShutdownTask is only
executed in tests where InitiateSamplingThreadIdleShutdown() is called.
Independent of that, I think this conditional can be removed entirely. See the
comment on the code.
If the previous conditional is removed, this can be tested by calling
InitiateSamplingThreadIdleShutdown(true) while a collection is in process.
We can't rely on the item being found for the for the same
configuration/load-dependent reasons as above.
This is pretty subtle in most of the tests but it's a key part of StopAfterIdle,
I think that's reasonable.
We can't rely on the item being not-found for the for the same
configuration/load-dependent reasons as above.


When run in their own, tests of the cases with configuration/load-dependent
sensitivity can have exactly the configuration they need to avoid races in the
behavior under test. They should be set up to be race-free even if the idle
shutdown is set to a very short duration.

[bcwhite, 2017/03/20 21:50:51]

I've disabled the idle shutdown in those tests.  60 seconds wasn't going to be
an issue but this is guaranteed.
For EXITING state specifically, the StopAfterIdle test will call Remove() when
it is in the EXITING state.
And wherever DisableIdleShutdown is not called.
There are dozens of stop calls before completion, some with huge timeouts. 
They're not all going to miss.
For all practical purposes, I believe we can.  Your original StopDuring*() tests
do this.

[Mike Wittman, 2017/03/21 16:50:38]

To repeat:
Relying on existing tests is OK if the test is obviously and reliably testing
the particular behavior. *All the other cases need dedicated tests* so that (1)
it's clear that the behavior is important and that it's actually being tested,
and (2) the test of the behavior survives changes and refactoring to the code
and tests.

The purpose in pointing out possible races is not to try to fix or explain away
the races. It's to demonstrate that the existing tests are not obviously and
reliably testing some of the behaviors, and that the test of those behaviors
needs to be split out into dedicated tests. Please create separate tests for
those behaviors.
I think this is true but it took me 10+ minutes of reasoning about the code to
reach that conclusion. So this is still not obvious and still needs a dedicated
test.

[bcwhite, 2017/03/22 17:48:54]

This IS the dedicated test!  StopAfterIdleShutdown was added, at your request,
just to cover this case.  The only way to know that a thread has gone into the
EXITING state is that it has stopped for being idle.  At any prior time, it
could still be a posted task waiting to execute.
I'll add a comment to that effect.
The run-time for the tasks is 1 day, longer than the run-time of the test.  If
the test completes, the task was stopped before completion.

[Mike Wittman, 2017/03/23 22:18:30]

This is still pretty subtle and could use some even more extensive comments
explaining how it works. I made suggestions in the code. 
It's still not obvious to a reader that this scenario is reliably tested. The
ConcurrentProfiling_* tests stated intention is to test behavior other than this
so could easily be changed in the future such that they no longer test this
behavior.

Please write a dedicated test. All that's required is starting a profiler with a
very large initial delay before starting a second profiler.
Please write a dedicated test.
 
None of these tests are intending to reliably exercise this behavior and could
easily be changed in the future such that they no longer test the behavior. The
use of Stop in the destructor is an implementation detail and shouldn't be
relied upon when testing this behavior.

Please write a dedicated test. All that's required is starting a profiler with a
very long sampling interval, waiting for one sample to be collected using the
test delegate, and stopping the profiler.
There's no reason to believe that the test process will continue execution long
enough for the next PerformCollectionTask to be executed after Stop. Even if
there was, none of these tests are intending to exercise this behavior and could
easily be changed in the future such that they no longer test the behavior.

Please write a dedicated test. This is trickier but I believe can be done by
relying on the message loop task ordering and observing when samples are taken
via the test delegate. Start two profilers with interleaved execution, wait for
both to take samples, stop the first, and observe that two samples of the second
occur with no interleaved sample from the first.

[bcwhite, 2017/03/27 17:52:43]

Done.
Done.  MultipleStart.
IdleShutdownAbort

It's definitely not that easy but I've changed the InitiateShutdown into a
PerformShutdown that waits for the task to execute.  At that point the test can
know that at least StopSoon() has been called and the state set to EXITING but
there's still no way to know if the thread has actually exited without waiting.
StopSafely

Thanks for being specific.  Done.
Done as part of StopSafely.

[Mike Wittman, 2017/03/28 19:32:01]

Please split this out into a dedicated test. It's not easy to even determine
that this behavior is being tested from reading StopSafely because of the
complexity required for the not found case of PerformCollectionTask.

[bcwhite, 2017/03/29 14:56:57]

That would be pretty much the same as StopDuringInterSampleInterval but using a
test delegate rather than timers.  That's what you want?

[Mike Wittman, 2017/03/30 16:18:38]

Yes. We might as well replace StopDuringInterSampleInterval's implementation
with that one, along with a comment specifying the additional behavior being
tested, since that's one of the existing tests that is racy.

[bcwhite, 2017/03/30 18:54:50]

Done.

   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 = 9;
 
-    params[1].sampling_interval = TimeDelta::FromMilliseconds(0);
-    params[1].samples_per_burst = 1;
+    params[1].initial_delay = TimeDelta::FromMilliseconds(11);
+    params[1].sampling_interval = TimeDelta::FromMilliseconds(1);
+    params[1].samples_per_burst = 8;
 
     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) {
+    std::vector<std::unique_ptr<WaitableEvent>> sampling_completed(
+        arraysize(params));
+    std::vector<std::unique_ptr<StackSamplingProfiler>> profiler(
+        arraysize(params));
+    for (size_t i = 0; i < arraysize(params); ++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);
     }
 
     profiler[0]->Start();
     profiler[1]->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);
+    size_t completed_profiler = WaitForSamplingComplete(&sampling_completed);
     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();
+    // Wait for the other profiler to finish.
     sampling_completed[other_profiler]->Wait();
     EXPECT_EQ(1u, profiles[other_profiler].size());
+
+    // Ensure each got the correct number of samples.
+    EXPECT_EQ(9u, profiles[0][0].samples.size());
+    EXPECT_EQ(8u, profiles[1][0].samples.size());
   });
 }
 
+// Checks that interleaved multiple sampling requests execute in parallel.
+#if defined(STACK_SAMPLING_PROFILER_SUPPORTED)
+#define MAYBE_ConcurrentProfiling_Interleaved ConcurrentProfiling_Interleaved
+#else
+#define MAYBE_ConcurrentProfiling_Interleaved \
+  DISABLED_ConcurrentProfiling_Interleaved
+#endif
+TEST(StackSamplingProfilerTest, MAYBE_ConcurrentProfiling_Interleaved) {
+  WithTargetThread([](PlatformThreadId target_thread_id) {
+    SamplingParams params[2];
+    params[0].initial_delay = TimeDelta::FromMilliseconds(1);
+    params[0].sampling_interval = TimeDelta::FromMilliseconds(2);
+    params[0].samples_per_burst = 10;
+
+    params[1].initial_delay = TimeDelta::FromMilliseconds(2);
+    params[1].sampling_interval = TimeDelta::FromMilliseconds(2);
+    params[1].samples_per_burst = 10;
+
+    CallStackProfiles profiles[2];
+    std::vector<std::unique_ptr<WaitableEvent>> sampling_completed(
+        arraysize(params));
+    std::vector<std::unique_ptr<StackSamplingProfiler>> profiler(
+        arraysize(params));
+    for (size_t i = 0; i < arraysize(params); ++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();
+
+    // Wait for one profiler to finish.
+    size_t completed_profiler = WaitForSamplingComplete(&sampling_completed);
+    EXPECT_EQ(1u, profiles[completed_profiler].size());
+    // Stop and destroy all profilers, always in the some order. Don't crash.

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

nit: same

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

Done.

+    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 several mixed sampling requests execute in parallel.
+#if defined(STACK_SAMPLING_PROFILER_SUPPORTED)
+#define MAYBE_ConcurrentProfiling_Mixed ConcurrentProfiling_Mixed
+#else
+#define MAYBE_ConcurrentProfiling_Mixed DISABLED_ConcurrentProfiling_Mixed
+#endif
+TEST(StackSamplingProfilerTest, MAYBE_ConcurrentProfiling_Mixed) {
+  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[arraysize(params)];
+    std::vector<std::unique_ptr<WaitableEvent>> sampling_completed(
+        arraysize(params));
+    std::vector<std::unique_ptr<StackSamplingProfiler>> profiler(
+        arraysize(params));
+    for (size_t i = 0; i < arraysize(params); ++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();
+
+    // Wait for one profiler to finish.
+    size_t completed_profiler = WaitForSamplingComplete(&sampling_completed);
+    EXPECT_EQ(1u, profiles[completed_profiler].size());
+    // Stop and destroy all profilers, always in the some order. Don't crash.

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

nit: same

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

Done.

+    for (size_t i = 0; i < profiler.size(); ++i)
+      profiler[i].reset();
+  });
+}
+
+// Checks that sampling requests execute in a staggered manner.
+#if defined(STACK_SAMPLING_PROFILER_SUPPORTED)
+#define MAYBE_ConcurrentProfiling_Staggered ConcurrentProfiling_Staggered
+#else
+#define MAYBE_ConcurrentProfiling_Staggered \
+  DISABLED_ConcurrentProfiling_Staggered
+#endif
+TEST(StackSamplingProfilerTest, MAYBE_ConcurrentProfiling_Staggered) {
+  WithTargetThread([](PlatformThreadId target_thread_id) {
+    SamplingParams params[3];
+    params[0].initial_delay = TimeDelta::FromMilliseconds(10);
+    params[0].sampling_interval = TimeDelta::FromMilliseconds(10);
+    params[0].samples_per_burst = 1;
+
+    params[1].initial_delay = TimeDelta::FromMilliseconds(5);
+    params[1].sampling_interval = TimeDelta::FromMilliseconds(10);
+    params[1].samples_per_burst = 2;
+
+    params[2].initial_delay = TimeDelta::FromMilliseconds(0);
+    params[2].sampling_interval = TimeDelta::FromMilliseconds(10);
+    params[2].samples_per_burst = 3;
+
+    CallStackProfiles profiles[arraysize(params)];
+    std::vector<std::unique_ptr<WaitableEvent>> sampling_completed(
+        arraysize(params));
+    std::vector<std::unique_ptr<StackSamplingProfiler>> profiler(
+        arraysize(params));
+    for (size_t i = 0; i < arraysize(params); ++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);
+    }
+
+    profiler[0]->Start();
+    profiler[1]->Start();
+    sampling_completed[0]->Wait();
+    EXPECT_FALSE(sampling_completed[1]->IsSignaled());
+    profiler[2]->Start();
+    profiler[0]->Stop();
+    profiler[1]->Stop();
+    sampling_completed[1]->Wait();
+    EXPECT_FALSE(sampling_completed[2]->IsSignaled());
+    sampling_completed[2]->Wait();
+    EXPECT_EQ(1u, profiles[0].size());
+    EXPECT_EQ(1u, profiles[1].size());
+    EXPECT_EQ(1u, profiles[2].size());
+  });
+}
+
 // Checks that a stack that runs through another library produces a stack with
 // the expected functions.
 #if defined(STACK_SAMPLING_PROFILER_SUPPORTED)
 #define MAYBE_OtherLibrary OtherLibrary
 #else
 #define MAYBE_OtherLibrary DISABLED_OtherLibrary
 #endif
 TEST(StackSamplingProfilerTest, MAYBE_OtherLibrary) {
   SamplingParams params;
   params.sampling_interval = TimeDelta::FromMilliseconds(0);
@@ skipping 74 matching lines @@
 #if defined(STACK_SAMPLING_PROFILER_SUPPORTED)
 #define MAYBE_UnloadedLibrary UnloadedLibrary
 #else
 #define MAYBE_UnloadedLibrary DISABLED_UnloadedLibrary
 #endif
 TEST(StackSamplingProfilerTest, MAYBE_UnloadedLibrary) {
   TestLibraryUnload(true);
 }
 
 }  // namespace base