[cpu-profiler] Wait on a condition variable in the sampling thread to enable quicker shutdowns

Signal a condition variable when profiling thread shutdown should occur,
waking up a profiling thread that's currently waiting for the next tick.

Mitigates the case where if a high sample interval is specified (e.g.
60s), the main thread is blocked until the next sample occurs due to a
Sleep() call.

Bug: v8:8843
Change-Id: Ied6b0bfb5c47a072ade17870911b961f5091f613
Reviewed-on: https://chromium-review.googlesource.com/c/1470953
Commit-Queue: Peter Marshall <petermarshall@chromium.org>
Reviewed-by: Peter Marshall <petermarshall@chromium.org>
Reviewed-by: Alexei Filippov <alph@chromium.org>
Cr-Commit-Position: refs/heads/master@{#59648}

src/profiler/cpu-profiler.cc

@@ -8,7 +8,6 @@
 #include <utility>
 
 #include "src/base/lazy-instance.h"
-#include "src/base/platform/mutex.h"
 #include "src/base/template-utils.h"
 #include "src/debug/debug.h"
 #include "src/deoptimizer.h"
@@ -108,6 +107,10 @@ void ProfilerEventsProcessor::AddSample(TickSample sample) {
 
 void ProfilerEventsProcessor::StopSynchronously() {
   if (!base::Relaxed_AtomicExchange(&running_, 0)) return;
+  {
+    base::MutexGuard guard(&running_mutex_);
+    running_cond_.NotifyOne();
+  }
   Join();
 }
 
@@ -179,6 +182,7 @@ SamplingEventsProcessor::ProcessOneSample() {
 }
 
 void SamplingEventsProcessor::Run() {
+  base::MutexGuard guard(&running_mutex_);
   while (!!base::Relaxed_Load(&running_)) {
     base::TimeTicks nextSampleTime =
         base::TimeTicks::HighResolutionNow() + period_;
@@ -206,7 +210,18 @@ void SamplingEventsProcessor::Run() {
       } else  // NOLINT
 #endif
       {
-        base::OS::Sleep(nextSampleTime - now);
+        // Allow another thread to interrupt the delay between samples in the
+        // event of profiler shutdown.
+        while (now < nextSampleTime &&
+               running_cond_.WaitFor(&running_mutex_, nextSampleTime - now)) {
+          // If true was returned, we got interrupted before the timeout
+          // elapsed. If this was not due to a change in running state, a
+          // spurious wakeup occurred (thus we should continue to wait).
+          if (!base::Relaxed_Load(&running_)) {
+            break;
+          }
+          now = base::TimeTicks::HighResolutionNow();
+        }
       }
     }
 

src/profiler/cpu-profiler.h

@@ -10,6 +10,8 @@
 #include "src/allocation.h"
 #include "src/base/atomic-utils.h"
 #include "src/base/atomicops.h"
+#include "src/base/platform/condition-variable.h"
+#include "src/base/platform/mutex.h"
 #include "src/base/platform/time.h"
 #include "src/isolate.h"
 #include "src/libsampler/sampler.h"
@@ -163,6 +165,8 @@ class ProfilerEventsProcessor : public base::Thread, public CodeEventObserver {
 
   ProfileGenerator* generator_;
   base::Atomic32 running_;
+  base::ConditionVariable running_cond_;
+  base::Mutex running_mutex_;
   LockedQueue<CodeEventsContainer> events_buffer_;
   LockedQueue<TickSampleEventRecord> ticks_from_vm_buffer_;
   std::atomic<unsigned> last_code_event_id_;

test/cctest/test-cpu-profiler.cc

@@ -2805,6 +2805,24 @@ TEST(MultipleIsolates) {
   thread2.Join();
 }
 
+// Tests that StopProfiling doesn't wait for the next sample tick in order to
+// stop, but rather exits early before a given wait threshold.
+TEST(FastStopProfiling) {
+  static const base::TimeDelta kLongInterval = base::TimeDelta::FromSeconds(10);
+  static const base::TimeDelta kWaitThreshold = base::TimeDelta::FromSeconds(5);
+
+  std::unique_ptr<CpuProfiler> profiler(new CpuProfiler(CcTest::i_isolate()));
+  profiler->set_sampling_interval(kLongInterval);
+  profiler->StartProfiling("", true);
+
+  v8::Platform* platform = v8::internal::V8::GetCurrentPlatform();
+  double start = platform->CurrentClockTimeMillis();
+  profiler->StopProfiling("");
+  double duration = platform->CurrentClockTimeMillis() - start;
+
+  CHECK_LT(duration, kWaitThreshold.InMillisecondsF());
+}
+
 int GetSourcePositionEntryCount(i::Isolate* isolate, const char* source) {
   i::Handle<i::JSFunction> function = i::Handle<i::JSFunction>::cast(
       v8::Utils::OpenHandle(*CompileRun(source)));