[cpu-profiler] Only record SIGPROF-based samples for samplers that request samples

Sets an atomic field on each sampler when it requests a sample, to be
checked when the SIGPROF handler is executed. A counter is not used
since signals may be coalesced.

Prior to this change, all samplers attached to an isolate received
samples when other samplers sent SIGPROF to the VM thread. This change
alters the behaviour of different CpuProfiler instances on the same
isolate to be in line with the Windows / Fuchsia behaviour.

Bug: v8:8835
Change-Id: I0caaa845b596efc9d8b1cd7716c067d9a6359c57
Reviewed-on: https://chromium-review.googlesource.com/c/1468941
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@{#59545}

src/libsampler/sampler.cc

@@ -234,6 +234,7 @@ void SamplerManager::DoSample(const v8::RegisterState& state) {
   SamplerList& samplers = it->second;
 
   for (Sampler* sampler : samplers) {
+    if (!sampler->ShouldRecordSample()) continue;
     Isolate* isolate = sampler->isolate();
     // We require a fully initialized and entered isolate.
     if (isolate == nullptr || !isolate->IsInUse()) continue;
@@ -542,6 +543,7 @@ void Sampler::Stop() {
 void Sampler::DoSample() {
   if (!SignalHandler::Installed()) return;
   DCHECK(IsActive());
+  SetShouldRecordSample();
   pthread_kill(platform_data()->vm_tid(), SIGPROF);
 }
 

src/libsampler/sampler.h

@@ -49,6 +49,12 @@ class Sampler {
   // Whether the sampler is running (start has been called).
   bool IsActive() const { return active_.load(std::memory_order_relaxed); }
 
+  // Returns true and consumes the pending sample bit if a sample should be
+  // dispatched to this sampler.
+  bool ShouldRecordSample() {
+    return record_sample_.exchange(false, std::memory_order_relaxed);
+  }
+
   void DoSample();
 
   // Used in tests to make sure that stack sampling is performed.
@@ -73,8 +79,13 @@ class Sampler {
     active_.store(value, std::memory_order_relaxed);
   }
 
+  void SetShouldRecordSample() {
+    record_sample_.store(true, std::memory_order_relaxed);
+  }
+
   Isolate* isolate_;
   std::atomic_bool active_{false};
+  std::atomic_bool record_sample_{false};
   std::unique_ptr<PlatformData> data_;  // Platform specific data.
   DISALLOW_IMPLICIT_CONSTRUCTORS(Sampler);
 };

test/cctest/libsampler/test-sampler.cc

@@ -141,6 +141,7 @@ class CountingSampler : public Sampler {
 
   int sample_count() { return sample_count_; }
   void set_active(bool active) { SetActive(active); }
+  void set_should_record_sample() { SetShouldRecordSample(); }
 
  private:
   int sample_count_ = 0;
@@ -153,6 +154,7 @@ TEST(SamplerManager_AddRemoveSampler) {
   SamplerManager* manager = SamplerManager::instance();
   CountingSampler sampler1(isolate);
   sampler1.set_active(true);
+  sampler1.set_should_record_sample();
   CHECK_EQ(0, sampler1.sample_count());
 
   manager->AddSampler(&sampler1);
@@ -162,6 +164,7 @@ TEST(SamplerManager_AddRemoveSampler) {
   CHECK_EQ(1, sampler1.sample_count());
 
   sampler1.set_active(true);
+  sampler1.set_should_record_sample();
   manager->RemoveSampler(&sampler1);
   sampler1.set_active(false);
 
@@ -177,6 +180,7 @@ TEST(SamplerManager_DoesNotReAdd) {
   SamplerManager* manager = SamplerManager::instance();
   CountingSampler sampler1(isolate);
   sampler1.set_active(true);
+  sampler1.set_should_record_sample();
   manager->AddSampler(&sampler1);
   manager->AddSampler(&sampler1);
 

test/cctest/test-cpu-profiler.cc

@@ -2721,6 +2721,38 @@ TEST(CrashReusedProfiler) {
   profiler->StopProfiling("2");
 }
 
+// Tests that samples from different profilers on the same isolate do not leak
+// samples to each other. See crbug.com/v8/8835.
+TEST(MultipleProfilersSampleIndependently) {
+  LocalContext env;
+  i::Isolate* isolate = CcTest::i_isolate();
+  i::HandleScope scope(isolate);
+
+  // Create two profilers- one slow ticking one, and one fast ticking one.
+  // Ensure that the slow ticking profiler does not receive samples from the
+  // fast ticking one.
+  std::unique_ptr<CpuProfiler> slow_profiler(
+      new CpuProfiler(CcTest::i_isolate()));
+  slow_profiler->set_sampling_interval(base::TimeDelta::FromSeconds(1));
+  slow_profiler->StartProfiling("1", true);
+
+  CompileRun(R"(
+    function start() {
+      let val = 1;
+      for (let i = 0; i < 10e3; i++) {
+        val = (val * 2) % 3;
+      }
+      return val;
+    }
+  )");
+  v8::Local<v8::Function> function = GetFunction(env.local(), "start");
+  ProfilerHelper helper(env.local());
+  v8::CpuProfile* profile = helper.Run(function, nullptr, 0, 100, 0, true);
+
+  auto slow_profile = slow_profiler->StopProfiling("1");
+  CHECK_GT(profile->GetSamplesCount(), slow_profile->samples_count());
+}
+
 void ProfileSomeCode(v8::Isolate* isolate) {
   v8::Isolate::Scope isolate_scope(isolate);
   v8::HandleScope scope(isolate);