http: fixup perf regression

Only call into hrtime if there's an observer Also, fix up some previously missed changes from the original refactor Signed-off-by: James M Snell <jasnell@gmail.com> Refs: #37937 Refs: #37136 PR-URL: #38110 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Richard Lau <rlau@redhat.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
nodejs · Apr 9, 2021 · 40ace47 · 40ace47
1 parent d2f116c
commit 40ace47
Show file tree

Hide file tree

Showing 4 changed files with 35 additions and 37 deletions.
diff --git a/lib/_http_server.js b/lib/_http_server.js
@@ -101,6 +101,10 @@ const onResponseFinishChannel = dc.channel('http.server.response.finish');
 const kServerResponse = Symbol('ServerResponse');
 const kServerResponseStatistics = Symbol('ServerResponseStatistics');
 
+const {
+  hasObserver,
+} = require('internal/perf/observe');
+
 const STATUS_CODES = {
   100: 'Continue',                   // RFC 7231 6.2.1
   101: 'Switching Protocols',        // RFC 7231 6.2.2
@@ -194,9 +198,11 @@ function ServerResponse(req) {
     this.shouldKeepAlive = false;
   }
 
-  this[kServerResponseStatistics] = {
-    startTime: process.hrtime()
-  };
+  if (hasObserver('http')) {
+    this[kServerResponseStatistics] = {
+      startTime: process.hrtime()
+    };
+  }
 }
 ObjectSetPrototypeOf(ServerResponse.prototype, OutgoingMessage.prototype);
 ObjectSetPrototypeOf(ServerResponse, OutgoingMessage);

diff --git a/lib/internal/http.js b/lib/internal/http.js
@@ -10,7 +10,11 @@ const {
 const { setUnrefTimeout } = require('internal/timers');
 
 const { InternalPerformanceEntry } = require('internal/perf/perf');
-const { enqueue } = require('internal/perf/observe');
+
+const {
+  enqueue,
+  hasObserver,
+} = require('internal/perf/observe');
 
 let utcCache;
 
@@ -30,6 +34,7 @@ function resetCache() {
 }
 
 function emitStatistics(statistics) {
+  if (!hasObserver('http') || statistics == null) return;
   const startTime = statistics.startTime;
   const diff = process.hrtime(startTime);
   const entry = new InternalPerformanceEntry(
@@ -46,5 +51,5 @@ module.exports = {
   kOutHeaders: Symbol('kOutHeaders'),
   kNeedDrain: Symbol('kNeedDrain'),
   utcDate,
-  emitStatistics
+  emitStatistics,
 };
diff --git a/lib/internal/perf/observe.js b/lib/internal/perf/observe.js
@@ -19,7 +19,6 @@ const {
 const {
   constants: {
     NODE_PERFORMANCE_ENTRY_TYPE_GC,
-    NODE_PERFORMANCE_ENTRY_TYPE_FUNCTION,
     NODE_PERFORMANCE_ENTRY_TYPE_HTTP2,
     NODE_PERFORMANCE_ENTRY_TYPE_HTTP,
   },
@@ -97,23 +96,18 @@ function queuePending() {
   });
 }
 
+function getObserverType(type) {
+  switch (type) {
+    case 'gc': return NODE_PERFORMANCE_ENTRY_TYPE_GC;
+    case 'http2': return NODE_PERFORMANCE_ENTRY_TYPE_HTTP2;
+    case 'http': return NODE_PERFORMANCE_ENTRY_TYPE_HTTP;
+  }
+}
+
 function maybeDecrementObserverCounts(entryTypes) {
   for (const type of entryTypes) {
-    let observerType;
-    switch (type) {
-      case 'gc':
-        observerType = NODE_PERFORMANCE_ENTRY_TYPE_GC;
-        break;
-      case 'function':
-        observerType = NODE_PERFORMANCE_ENTRY_TYPE_FUNCTION;
-        break;
-      case 'http2':
-        observerType = NODE_PERFORMANCE_ENTRY_TYPE_HTTP2;
-        break;
-      case 'http':
-        observerType = NODE_PERFORMANCE_ENTRY_TYPE_HTTP;
-        break;
-    }
+    const observerType = getObserverType(type);
+
     if (observerType !== undefined) {
       observerCounts[observerType]--;
 
@@ -126,24 +120,10 @@ function maybeDecrementObserverCounts(entryTypes) {
 }
 
 function maybeIncrementObserverCount(type) {
-  let observerType;
-  switch (type) {
-    case 'gc':
-      observerType = NODE_PERFORMANCE_ENTRY_TYPE_GC;
-      break;
-    case 'function':
-      observerType = NODE_PERFORMANCE_ENTRY_TYPE_FUNCTION;
-      break;
-    case 'http2':
-      observerType = NODE_PERFORMANCE_ENTRY_TYPE_HTTP2;
-      break;
-    case 'http':
-      observerType = NODE_PERFORMANCE_ENTRY_TYPE_HTTP;
-      break;
-  }
+  const observerType = getObserverType(type);
+
   if (observerType !== undefined) {
     observerCounts[observerType]++;
-
     if (observerType === NODE_PERFORMANCE_ENTRY_TYPE_GC)
       installGarbageCollectionTracking();
   }
@@ -346,7 +326,13 @@ function observerCallback(name, type, startTime, duration, details) {
 
 setupObservers(observerCallback);
 
+function hasObserver(type) {
+  const observerType = getObserverType(type);
+  return observerCounts[observerType] > 0;
+}
+
 module.exports = {
   PerformanceObserver,
   enqueue,
+  hasObserver,
 };
diff --git a/src/node_perf_common.h b/src/node_perf_common.h
@@ -33,6 +33,7 @@ extern uint64_t performance_v8_start;
 
 #define NODE_PERFORMANCE_ENTRY_TYPES(V)                                       \
   V(GC, "gc")                                                                 \
+  V(HTTP, "http")                                                             \
   V(HTTP2, "http2")
 
 enum PerformanceMilestone {