Suggestions from Nicolas' review, minus the atomic update of profilers map

Author: szegedi

bindings/profilers/wall.cc

@@ -363,7 +363,7 @@ LabelSetsByNode WallProfiler::GetLabelSetsByNode(CpuProfile* profile,
           return labelSetsByNode;
         }
         sampleContext = contexts.pop_front();
-        break; // don't match more than one context to one sample
+        break;  // don't match more than one context to one sample
       }
     }
   }
@@ -373,8 +373,6 @@ LabelSetsByNode WallProfiler::GetLabelSetsByNode(CpuProfile* profile,
   return labelSetsByNode;
 }
 
-static Nan::Persistent<v8::Function> constructor;
-
 WallProfiler::WallProfiler(int intervalMicros, int durationMicros)
     : samplingInterval(intervalMicros),
       contexts(durationMicros * 2 / intervalMicros) {}
@@ -438,7 +436,7 @@ NAN_METHOD(WallProfiler::New) {
     return Nan::ThrowTypeError("Sample rate must be a number.");
   }
 
-  if (!info[0]->IsNumber()) {
+  if (!info[1]->IsNumber()) {
     return Nan::ThrowTypeError("Duration must be a number.");
   }
 
@@ -464,8 +462,8 @@ NAN_METHOD(WallProfiler::New) {
     obj->Wrap(info.This());
     info.GetReturnValue().Set(info.This());
   } else {
-    const int argc = 1;
-    v8::Local<v8::Value> argv[argc] = {info[0]};
+    const int argc = 2;
+    v8::Local<v8::Value> argv[argc] = {info[0], info[1]};
     v8::Local<v8::Function> cons = Nan::New(
         PerIsolateData::For(info.GetIsolate())->WallProfilerConstructor());
     info.GetReturnValue().Set(

bindings/profilers/wall.hh

@@ -2,7 +2,9 @@
 
 #include <nan.h>
 #include <v8-profiler.h>
+#include <memory>
 #include <unordered_map>
+#include <utility>
 
 #include "../buffer.hh"
 
@@ -28,10 +30,9 @@ class WallProfiler : public Nan::ObjectWrap {
     uint64_t timestamp;
 
     // Needed to initialize ring buffer elements
-    SampleContext() {}
+    SampleContext() = default;
 
-    SampleContext(std::shared_ptr<v8::Global<v8::Value>> l, uint64_t t)
-        : labels(l), timestamp(t) {}
+    SampleContext(const ValuePtr& l, uint64_t t) : labels(l), timestamp(t) {}
   };
 
   RingBuffer<SampleContext> contexts;
@@ -58,11 +59,7 @@ class WallProfiler : public Nan::ObjectWrap {
 
   v8::Local<v8::Value> GetLabels(v8::Isolate*);
   void SetLabels(v8::Isolate*, v8::Local<v8::Value>);
-  bool GetLabelsCaptured() {
-    bool captured = labelsCaptured;
-    labelsCaptured = false;
-    return captured;
-  }
+  bool GetLabelsCaptured() { return std::exchange(labelsCaptured, false); }
 
   uint64_t PushContext();
   void StartImpl(v8::Local<v8::String> name,

ts/src/time-profiler.ts

@@ -22,7 +22,7 @@ import {TimeProfiler} from './time-profiler-bindings';
 import {LabelSet} from './v8-types';
 
 const DEFAULT_INTERVAL_MICROS: Microseconds = 1000;
-const DEFAULT_DURATION_MILLIS: Milliseconds = 60000;
+const DEFAULT_DURATION_MICROS: Microseconds = 60000000;
 
 const majorVersion = process.version.slice(1).split('.').map(Number)[0];
 
@@ -83,14 +83,14 @@ export function start(
 
 export function startWithLabels(
   intervalMicros: Microseconds = DEFAULT_INTERVAL_MICROS,
-  durationMillis: Milliseconds = DEFAULT_DURATION_MILLIS,
+  durationMicros: Microseconds = DEFAULT_DURATION_MICROS,
   name?: string,
   sourceMapper?: SourceMapper,
   lineNumbers = true
 ) {
   return startInternal(
     intervalMicros,
-    durationMillis * 1000,
+    durationMicros,
     name,
     sourceMapper,
     lineNumbers,

ts/test/test-time-profiler.ts

@@ -50,7 +50,7 @@ describe('Time Profiler', () => {
       const intervalNanos = PROFILE_OPTIONS.intervalMicros * 1_000;
       const {stop, setLabels} = time.startWithLabels(
         PROFILE_OPTIONS.intervalMicros,
-        PROFILE_OPTIONS.durationMillis
+        PROFILE_OPTIONS.durationMillis * 1000
       );
       const repeats = 3;
       // By repeating the test few times, we can also test the overlap behavior.