Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(547)

Unified Diff: base/profiler/stack_sampling_profiler_unittest.cc

Issue 2554123002: Support parallel captures from the StackSamplingProfiler. (Closed)
Patch Set: addressed review comments by wittman Created 3 years, 10 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
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..1bea7f200898eeb6150389b2da8654892faff724 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,8 @@ TEST(StackSamplingProfilerTest, MAYBE_DestroyProfilerWhileProfiling) {
#define MAYBE_CanRunMultipleTimes DISABLED_CanRunMultipleTimes
#endif
TEST(StackSamplingProfilerTest, MAYBE_CanRunMultipleTimes) {
+ StackSamplingProfiler::TestAPI::SetSamplingThreadIdleShutdownTime(0);
+
SamplingParams params;
params.sampling_interval = TimeDelta::FromMilliseconds(0);
params.samples_per_burst = 1;
@@ -871,10 +844,24 @@ TEST(StackSamplingProfilerTest, MAYBE_CanRunMultipleTimes) {
profiles.clear();
CaptureProfiles(params, AVeryLongTimeDelta(), &profiles);
ASSERT_EQ(1u, profiles.size());
+
+ // Capture thread should still be running at this point.
Mike Wittman 2017/02/22 03:06:48 The shutdown and restart behavior should be broken
bcwhite 2017/02/22 14:32:51 Done.
+ 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();
+
+ // 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 requests to start profiling while another profile is taking place
-// are ignored.
+// Checks that multiple sampling requests execute in parallel.
#if defined(STACK_SAMPLING_PROFILER_SUPPORTED)
#define MAYBE_ConcurrentProfiling ConcurrentProfiling
Mike Wittman 2017/02/22 03:06:48 I think we should test different interleavings of
bcwhite 2017/02/22 14:32:51 Done.
#else
@@ -884,11 +871,12 @@ TEST(StackSamplingProfilerTest, MAYBE_ConcurrentProfiling) {
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[0].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 +907,9 @@ 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)));
});
}

Powered by Google App Engine
This is Rietveld 408576698