Move profiler start/stop/restart logic into native code.

Make ring buffer operations interrupt safe.

Author: szegedi

bindings/profilers/wall.cc

@@ -61,19 +61,17 @@ static void sighandler(int sig, siginfo_t* info, void* context) {
   }
 
   auto isolate = Isolate::GetCurrent();
-  WallProfiler* prof = nullptr;
   auto prof_map = profilers.exchange(nullptr, std::memory_order_acq_rel);
   if (prof_map) {
+    WallProfiler* prof = nullptr;
     auto prof_it = prof_map->find(isolate);
     if (prof_it != prof_map->end()) {
       prof = prof_it->second;
     }
     profilers.store(prof_map, std::memory_order_release);
-  }
-  auto now = v8::base::TimeTicks::Now();
-  old_handler(sig, info, context);
-  if (prof) {
-    prof->PushContext(now);
+    if (prof) {
+      prof->DoSignalHandler(sig, info, context);
+    }
   }
 }
 #endif
@@ -301,7 +299,8 @@ bool isIdleSample(const CpuProfileNode* sample) {
       strncmp("(idle)", sample->GetFunctionNameStr(), 7) == 0;
 }
 
-LabelSetsByNode WallProfiler::GetLabelSetsByNode(CpuProfile* profile) {
+LabelSetsByNode WallProfiler::GetLabelSetsByNode(
+    CpuProfile* profile, RingBuffer<SampleContext>& contexts) {
   LabelSetsByNode labelSetsByNode;
 
   auto sampleCount = profile->GetSamplesCount();
@@ -356,12 +355,21 @@ LabelSetsByNode WallProfiler::GetLabelSetsByNode(CpuProfile* profile) {
       }
     }
   }
+  while (!contexts.empty()) {
+    contexts.pop_front();
+  }
   return labelSetsByNode;
 }
 
-WallProfiler::WallProfiler(int intervalMicros, int durationMicros)
+WallProfiler::WallProfiler(int intervalMicros,
+                           int durationMicros,
+                           bool includeLines_,
+                           bool withLabels_)
     : samplingInterval(intervalMicros),
-      contexts(durationMicros * 2 / intervalMicros) {}
+      includeLines(includeLines_),
+      withLabels(withLabels_),
+      session1(durationMicros * 2 / intervalMicros),
+      session2(durationMicros * 2 / intervalMicros) {}
 
 WallProfiler::~WallProfiler() {
   Dispose(nullptr);
@@ -414,16 +422,9 @@ void WallProfiler::Dispose(Isolate* isolate) {
   }
 }
 
-NAN_METHOD(WallProfiler::Dispose) {
-  WallProfiler* wallProfiler =
-      Nan::ObjectWrap::Unwrap<WallProfiler>(info.Holder());
-
-  wallProfiler->Dispose(info.GetIsolate());
-}
-
 NAN_METHOD(WallProfiler::New) {
-  if (info.Length() != 2) {
-    return Nan::ThrowTypeError("WallProfiler must have two arguments.");
+  if (info.Length() != 4) {
+    return Nan::ThrowTypeError("WallProfiler must have four arguments.");
   }
   if (!info[0]->IsNumber()) {
     return Nan::ThrowTypeError("Sample rate must be a number.");
@@ -433,6 +434,14 @@ NAN_METHOD(WallProfiler::New) {
     return Nan::ThrowTypeError("Duration must be a number.");
   }
 
+  if (!info[2]->IsBoolean()) {
+    return Nan::ThrowTypeError("includeLines must be a boolean.");
+  }
+
+  if (!info[3]->IsBoolean()) {
+    return Nan::ThrowTypeError("withLabels must be a boolean.");
+  }
+
   if (info.IsConstructCall()) {
     int interval = Nan::MaybeLocal<Integer>(info[0].As<Integer>())
                        .ToLocalChecked()
@@ -451,12 +460,21 @@ NAN_METHOD(WallProfiler::New) {
       return Nan::ThrowTypeError("Duration must not be less than sample rate.");
     }
 
-    WallProfiler* obj = new WallProfiler(interval, duration);
+    bool includeLines = Nan::MaybeLocal<Boolean>(info[2].As<Boolean>())
+                            .ToLocalChecked()
+                            ->Value();
+
+    bool withLabels = Nan::MaybeLocal<Boolean>(info[3].As<Boolean>())
+                          .ToLocalChecked()
+                          ->Value();
+
+    WallProfiler* obj =
+        new WallProfiler(interval, duration, includeLines, withLabels);
     obj->Wrap(info.This());
     info.GetReturnValue().Set(info.This());
   } else {
-    const int argc = 2;
-    v8::Local<v8::Value> argv[argc] = {info[0], info[1]};
+    const int argc = 4;
+    v8::Local<v8::Value> argv[argc] = {info[0], info[1], info[2], info[3]};
     v8::Local<v8::Function> cons = Nan::New(
         PerIsolateData::For(info.GetIsolate())->WallProfilerConstructor());
     info.GetReturnValue().Set(
@@ -468,93 +486,101 @@ NAN_METHOD(WallProfiler::Start) {
   WallProfiler* wallProfiler =
       Nan::ObjectWrap::Unwrap<WallProfiler>(info.Holder());
 
-  if (info.Length() != 3) {
-    return Nan::ThrowTypeError("Start must have three arguments.");
-  }
-  if (!info[0]->IsString()) {
-    return Nan::ThrowTypeError("Profile name must be a string.");
-  }
-  if (!info[1]->IsBoolean()) {
-    return Nan::ThrowTypeError("Include lines flag must be a boolean.");
-  }
-  if (!info[2]->IsBoolean()) {
-    return Nan::ThrowTypeError("With labels flag must be a boolean.");
+  if (info.Length() != 0) {
+    return Nan::ThrowTypeError("Start must have no arguments.");
   }
 
-  Local<String> name =
-      Nan::MaybeLocal<String>(info[0].As<String>()).ToLocalChecked();
+  return wallProfiler->StartStop(true, false, info);
+}
 
-  bool includeLines =
-      Nan::MaybeLocal<Boolean>(info[1].As<Boolean>()).ToLocalChecked()->Value();
+NAN_METHOD(WallProfiler::Stop) {
+  if (info.Length() != 1) {
+    return Nan::ThrowTypeError("Stop must have one arguments.");
+  }
+  if (!info[0]->IsBoolean()) {
+    return Nan::ThrowTypeError("restart must be a boolean.");
+  }
+  bool restart =
+      Nan::MaybeLocal<Boolean>(info[0].As<Boolean>()).ToLocalChecked()->Value();
 
-  bool withLabels =
-      Nan::MaybeLocal<Boolean>(info[2].As<Boolean>()).ToLocalChecked()->Value();
+  WallProfiler* wallProfiler =
+      Nan::ObjectWrap::Unwrap<WallProfiler>(info.Holder());
 
-  auto now = v8::base::TimeTicks::Now();
-  wallProfiler->StartImpl(name, includeLines, withLabels);
-#ifdef DD_WALL_USE_SIGPROF
-  if (withLabels) {
-    wallProfiler->PushContext(now);
-    struct sigaction sa, old_sa;
-    sa.sa_flags = SA_SIGINFO | SA_RESTART;
-    sa.sa_sigaction = &sighandler;
-    sigemptyset(&sa.sa_mask);
-    sigaction(SIGPROF, &sa, &old_sa);
-
-    // At the end of a cycle start is called before stop,
-    // at this point old_sa.sa_sigaction is sighandler !
-    if (!old_handler) {
-      old_handler = old_sa.sa_sigaction;
-    }
+  wallProfiler->StartStop(restart, true, info);
+  if (!restart) {
+    wallProfiler->Dispose(info.GetIsolate());
   }
-#endif
 }
 
-void WallProfiler::StartImpl(Local<String> name,
-                             bool includeLines,
-                             bool withLabels) {
-  if (includeLines) {
-    GetProfiler()->StartProfiling(
-        name, CpuProfilingMode::kCallerLineNumbers, withLabels);
-  } else {
-    GetProfiler()->StartProfiling(name, withLabels);
-  }
-}
+Nan::NAN_METHOD_RETURN_TYPE WallProfiler::StartStop(
+    bool start, bool stop, Nan::NAN_METHOD_ARGS_TYPE info) {
+  auto prevSession =
+      currentSession.exchange(nullptr, std::memory_order_relaxed);
+  std::atomic_signal_fence(std::memory_order_release);
 
-NAN_METHOD(WallProfiler::Stop) {
-  if (info.Length() != 2) {
-    return Nan::ThrowTypeError("Stop must have two arguments.");
-  }
-  if (!info[0]->IsString()) {
-    return Nan::ThrowTypeError("Profile name must be a string.");
+  if (start && !stop && prevSession) {
+    currentSession.store(prevSession, std::memory_order_relaxed);
+    std::atomic_signal_fence(std::memory_order_release);
+    return Nan::ThrowError("Start called with already started profiling "
+                           "session. Call stop first.");
   }
-  if (!info[1]->IsBoolean()) {
-    return Nan::ThrowTypeError("Include lines must be a boolean.");
+
+  if (stop && !prevSession) {
+    return Nan::ThrowError(
+        "Stop called without started profiling session. Call start first.");
   }
 
-  Local<String> name =
-      Nan::MaybeLocal<String>(info[0].As<String>()).ToLocalChecked();
+  ProfilingSession* nextSession = nullptr;
+  if (start) {
+    nextSession = prevSession == &session2 ? &session1 : &session2;
 
-  bool includeLines =
-      Nan::MaybeLocal<Boolean>(info[1].As<Boolean>()).ToLocalChecked()->Value();
+    auto now = v8::base::TimeTicks::Now();
+    auto nextNameLocal = Nan::New<String>(std::to_string(now)).ToLocalChecked();
+    nextSession->name.Reset(info.GetIsolate(), nextNameLocal);
+    if (includeLines) {
+      GetProfiler()->StartProfiling(
+          nextNameLocal, CpuProfilingMode::kCallerLineNumbers, withLabels);
+    } else {
+      GetProfiler()->StartProfiling(nextNameLocal, withLabels);
+    }
 
-  WallProfiler* wallProfiler =
-      Nan::ObjectWrap::Unwrap<WallProfiler>(info.Holder());
+#ifdef DD_WALL_USE_SIGPROF
+    if (withLabels) {
+      auto labels = curLabels.load(std::memory_order_relaxed);
+      nextSession->contexts.push_back(
+          SampleContext(*labels, now, v8::base::TimeTicks::Now()));
+
+      struct sigaction sa, old_sa;
+      sa.sa_flags = SA_SIGINFO | SA_RESTART;
+      sa.sa_sigaction = &sighandler;
+      sigemptyset(&sa.sa_mask);
+      sigaction(SIGPROF, &sa, &old_sa);
+
+      if (!old_handler) {
+        old_handler = old_sa.sa_sigaction;
+      }
+    }
+#endif
+  }
 
-  auto profile = wallProfiler->StopImpl(name, includeLines);
+  CpuProfile* v8_profile = nullptr;
 
-  info.GetReturnValue().Set(profile);
-}
+  if (stop) {
+    v8_profile =
+        GetProfiler()->StopProfiling(prevSession->name.Get(info.GetIsolate()));
+    prevSession->name.Reset();
+  }
+
+  currentSession.store(nextSession, std::memory_order_relaxed);
+  std::atomic_signal_fence(std::memory_order_release);
 
-Local<Value> WallProfiler::StopImpl(Local<String> name,
-                                    bool includeLines) {
-  auto profiler = GetProfiler();
-  auto v8_profile = profiler->StopProfiling(name);
-  Local<Value> profile =
-      ProfileTranslator(GetLabelSetsByNode(v8_profile))
-          .TranslateTimeProfile(v8_profile, includeLines);
-  v8_profile->Delete();
-  return profile;
+  if (v8_profile) {
+    Local<Value> profile =
+        ProfileTranslator(GetLabelSetsByNode(v8_profile, prevSession->contexts))
+            .TranslateTimeProfile(v8_profile, includeLines);
+    v8_profile->Delete();
+    info.GetReturnValue().Set(profile);
+  }
 }
 
 NAN_MODULE_INIT(WallProfiler::Init) {
@@ -569,7 +595,6 @@ NAN_MODULE_INIT(WallProfiler::Init) {
                    SetLabels);
 
   Nan::SetPrototypeMethod(tpl, "start", Start);
-  Nan::SetPrototypeMethod(tpl, "dispose", Dispose);
   Nan::SetPrototypeMethod(tpl, "stop", Stop);
 
   PerIsolateData::For(Isolate::GetCurrent())
@@ -622,13 +647,17 @@ NAN_SETTER(WallProfiler::SetLabels) {
   profiler->SetLabels(info.GetIsolate(), value);
 }
 
-void WallProfiler::PushContext(int64_t time_from) {
-  // Be careful this is called in a signal handler context therefore all
-  // operations must be async signal safe (in particular no allocations). Our
-  // ring buffer avoids allocations.
-  auto labels = curLabels.load(std::memory_order_relaxed);
+void WallProfiler::DoSignalHandler(int sig, siginfo_t* info, void* context) {
+  auto currSession = currentSession.load(std::memory_order_relaxed);
   std::atomic_signal_fence(std::memory_order_acquire);
-  contexts.push_back(SampleContext(*labels, time_from, v8::base::TimeTicks::Now()));
+  if (currSession) {
+    auto labels = curLabels.load(std::memory_order_relaxed);
+    std::atomic_signal_fence(std::memory_order_acquire);
+    auto start = v8::base::TimeTicks::Now();
+    old_handler(sig, info, context);
+    currSession->contexts.push_back(
+        SampleContext(*labels, start, v8::base::TimeTicks::Now()));
+  }
 }
 
 }  // namespace dd

bindings/profilers/wall.hh

@@ -18,6 +18,8 @@ using ValuePtr = std::shared_ptr<v8::Global<v8::Value>>;
 class WallProfiler : public Nan::ObjectWrap {
  private:
   int samplingInterval = 0;
+  bool includeLines;
+  bool withLabels;
   v8::CpuProfiler* cpuProfiler = nullptr;
   // TODO: Investigate use of v8::Persistent instead of shared_ptr<Global> to
   // avoid heap allocation. Need to figure out the right move/copy semantics in
@@ -37,10 +39,20 @@ class WallProfiler : public Nan::ObjectWrap {
     // Needed to initialize ring buffer elements
     SampleContext() = default;
 
-    SampleContext(const ValuePtr& l, int64_t from, int64_t to) : labels(l), time_from(from), time_to(to) {}
+    SampleContext(const ValuePtr& l, int64_t from, int64_t to)
+        : labels(l), time_from(from), time_to(to) {}
   };
 
-  RingBuffer<SampleContext> contexts;
+  struct ProfilingSession {
+    ProfilingSession(int contextsCapacity) : contexts(contextsCapacity) {}
+
+    v8::Global<v8::String> name;
+    RingBuffer<SampleContext> contexts;
+  };
+
+  ProfilingSession session1;
+  ProfilingSession session2;
+  std::atomic<ProfilingSession*> currentSession = nullptr;
 
   ~WallProfiler();
   void Dispose(v8::Isolate* isolate);
@@ -49,7 +61,8 @@ class WallProfiler : public Nan::ObjectWrap {
   // to work around https://bugs.chromium.org/p/v8/issues/detail?id=11051.
   v8::CpuProfiler* GetProfiler();
 
-  LabelSetsByNode GetLabelSetsByNode(v8::CpuProfile* profile);
+  LabelSetsByNode GetLabelSetsByNode(v8::CpuProfile* profile,
+                                     RingBuffer<SampleContext>& contexts);
 
  public:
   /**
@@ -59,20 +72,20 @@ class WallProfiler : public Nan::ObjectWrap {
    * every period. The parameter is used to preallocate data structures that
    * should not be reallocated in async signal safe code.
    */
-  explicit WallProfiler(int intervalMicros, int durationMicros);
+  explicit WallProfiler(int intervalMicros,
+                        int durationMicros,
+                        bool includeLines,
+                        bool withLabels);
 
   v8::Local<v8::Value> GetLabels(v8::Isolate*);
   void SetLabels(v8::Isolate*, v8::Local<v8::Value>);
 
-  void PushContext(int64_t time_from);
-  void StartImpl(v8::Local<v8::String> name,
-                 bool includeLines,
-                 bool withLabels);
-  v8::Local<v8::Value> StopImpl(v8::Local<v8::String> name,
-                                bool includeLines);
+  void DoSignalHandler(int sig, siginfo_t* info, void* context);
+  Nan::NAN_METHOD_RETURN_TYPE StartStop(bool start,
+                                        bool stop,
+                                        Nan::NAN_METHOD_ARGS_TYPE info);
 
   static NAN_METHOD(New);
-  static NAN_METHOD(Dispose);
   static NAN_METHOD(Start);
   static NAN_METHOD(Stop);
   static NAN_MODULE_INIT(Init);