net,dns: move hasObserver out of perf function

move the hasObserver out of startPerf and stopPerf
to avoid generating useless objects when these are no observer

PR-URL: https://github.com/nodejs/node/pull/43217
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Minwoo Jung <nodecorelab@gmail.com>

4 files changed, 79 insertions, 52 deletions

diff --git a/lib/dns.js b/lib/dns.js
index 2e1a37b0818..ae908d46333 100644
--- a/lib/dns.js
+++ b/lib/dns.js
@@ -70,6 +70,7 @@ const kPerfHooksDnsLookupServiceContext = Symbol('kPerfHooksDnsLookupServiceCont
 const kPerfHooksDnsLookupResolveContext = Symbol('kPerfHooksDnsLookupResolveContext');
 
 const {
+  hasObserver,
   startPerf,
   stopPerf,
 } = require('internal/perf/observe');
@@ -83,7 +84,9 @@ function onlookup(err, addresses) {
     return this.callback(dnsException(err, 'getaddrinfo', this.hostname));
   }
   this.callback(null, addresses[0], this.family || isIP(addresses[0]));
-  stopPerf(this, kPerfHooksDnsLookupContext);
+  if (this[kPerfHooksDnsLookupContext] && hasObserver('dns')) {
+    stopPerf(this, kPerfHooksDnsLookupContext);
+  }
 }
 
 
@@ -102,7 +105,9 @@ function onlookupall(err, addresses) {
   }
 
   this.callback(null, addresses);
-  stopPerf(this, kPerfHooksDnsLookupContext);
+  if (this[kPerfHooksDnsLookupContext] && hasObserver('dns')) {
+    stopPerf(this, kPerfHooksDnsLookupContext);
+  }
 }
 
 
@@ -187,13 +192,15 @@ function lookup(hostname, options, callback) {
     process.nextTick(callback, dnsException(err, 'getaddrinfo', hostname));
     return {};
   }
-  const detail = {
-    hostname,
-    family,
-    hints,
-    verbatim,
-  };
-  startPerf(req, kPerfHooksDnsLookupContext, { type: 'dns', name: 'lookup', detail });
+  if (hasObserver('dns')) {
+    const detail = {
+      hostname,
+      family,
+      hints,
+      verbatim,
+    };
+    startPerf(req, kPerfHooksDnsLookupContext, { type: 'dns', name: 'lookup', detail });
+  }
   return req;
 }
 
@@ -206,7 +213,9 @@ function onlookupservice(err, hostname, service) {
     return this.callback(dnsException(err, 'getnameinfo', this.hostname));
 
   this.callback(null, hostname, service);
-  stopPerf(this, kPerfHooksDnsLookupServiceContext);
+  if (this[kPerfHooksDnsLookupServiceContext] && hasObserver('dns')) {
+    stopPerf(this, kPerfHooksDnsLookupServiceContext);
+  }
 }
 
 
@@ -231,14 +240,16 @@ function lookupService(address, port, callback) {
 
   const err = cares.getnameinfo(req, address, port);
   if (err) throw dnsException(err, 'getnameinfo', address);
-  startPerf(req, kPerfHooksDnsLookupServiceContext, {
-    type: 'dns',
-    name: 'lookupService',
-    detail: {
-      host: address,
-      port
-    }
-  });
+  if (hasObserver('dns')) {
+    startPerf(req, kPerfHooksDnsLookupServiceContext, {
+      type: 'dns',
+      name: 'lookupService',
+      detail: {
+        host: address,
+        port,
+      },
+    });
+  }
   return req;
 }
 
@@ -255,7 +266,9 @@ function onresolve(err, result, ttls) {
     this.callback(dnsException(err, this.bindingName, this.hostname));
   else {
     this.callback(null, result);
-    stopPerf(this, kPerfHooksDnsLookupResolveContext);
+    if (this[kPerfHooksDnsLookupResolveContext] && hasObserver('dns')) {
+      stopPerf(this, kPerfHooksDnsLookupResolveContext);
+    }
   }
 }
 
@@ -278,14 +291,16 @@ function resolver(bindingName) {
     req.ttl = !!(options && options.ttl);
     const err = this._handle[bindingName](req, toASCII(name));
     if (err) throw dnsException(err, bindingName, name);
-    startPerf(req, kPerfHooksDnsLookupResolveContext, {
-      type: 'dns',
-      name: bindingName,
-      detail: {
-        host: name,
-        ttl: req.ttl
-      }
-    });
+    if (hasObserver('dns')) {
+      startPerf(req, kPerfHooksDnsLookupResolveContext, {
+        type: 'dns',
+        name: bindingName,
+        detail: {
+          host: name,
+          ttl: req.ttl,
+        },
+      });
+    }
     return req;
   }
   ObjectDefineProperty(query, 'name', { __proto__: null, value: bindingName });
diff --git a/lib/internal/dns/promises.js b/lib/internal/dns/promises.js
index 9591c5826a5..58493134221 100644
--- a/lib/internal/dns/promises.js
+++ b/lib/internal/dns/promises.js
@@ -46,6 +46,7 @@ const kPerfHooksDnsLookupServiceContext = Symbol('kPerfHooksDnsLookupServiceCont
 const kPerfHooksDnsLookupResolveContext = Symbol('kPerfHooksDnsLookupResolveContext');
 
 const {
+  hasObserver,
   startPerf,
   stopPerf,
 } = require('internal/perf/observe');
@@ -58,7 +59,9 @@ function onlookup(err, addresses) {
 
   const family = this.family || isIP(addresses[0]);
   this.resolve({ address: addresses[0], family });
-  stopPerf(this, kPerfHooksDnsLookupContext);
+  if (this[kPerfHooksDnsLookupContext] && hasObserver('dns')) {
+    stopPerf(this, kPerfHooksDnsLookupContext);
+  }
 }
 
 function onlookupall(err, addresses) {
@@ -79,7 +82,9 @@ function onlookupall(err, addresses) {
   }
 
   this.resolve(addresses);
-  stopPerf(this, kPerfHooksDnsLookupContext);
+  if (this[kPerfHooksDnsLookupContext] && hasObserver('dns')) {
+    stopPerf(this, kPerfHooksDnsLookupContext);
+  }
 }
 
 function createLookupPromise(family, hostname, all, hints, verbatim) {
@@ -110,7 +115,7 @@ function createLookupPromise(family, hostname, all, hints, verbatim) {
 
     if (err) {
       reject(dnsException(err, 'getaddrinfo', hostname));
-    } else {
+    } else if (hasObserver('dns')) {
       const detail = {
         hostname,
         family,
@@ -170,7 +175,9 @@ function onlookupservice(err, hostname, service) {
   }
 
   this.resolve({ hostname, service });
-  stopPerf(this, kPerfHooksDnsLookupServiceContext);
+  if (this[kPerfHooksDnsLookupServiceContext] && hasObserver('dns')) {
+    stopPerf(this, kPerfHooksDnsLookupServiceContext);
+  }
 }
 
 function createLookupServicePromise(hostname, port) {
@@ -187,7 +194,7 @@ function createLookupServicePromise(hostname, port) {
 
     if (err)
       reject(dnsException(err, 'getnameinfo', hostname));
-    else
+    else if (hasObserver('dns')) {
       startPerf(req, kPerfHooksDnsLookupServiceContext, {
         type: 'dns',
         name: 'lookupService',
@@ -196,6 +203,7 @@ function createLookupServicePromise(hostname, port) {
           port
         }
       });
+    }
   });
 }
 
@@ -223,7 +231,9 @@ function onresolve(err, result, ttls) {
       result, (address, index) => ({ address, ttl: ttls[index] }));
 
   this.resolve(result);
-  stopPerf(this, kPerfHooksDnsLookupResolveContext);
+  if (this[kPerfHooksDnsLookupResolveContext] && hasObserver('dns')) {
+    stopPerf(this, kPerfHooksDnsLookupResolveContext);
+  }
 }
 
 function createResolverPromise(resolver, bindingName, hostname, ttl) {
@@ -241,7 +251,7 @@ function createResolverPromise(resolver, bindingName, hostname, ttl) {
 
     if (err)
       reject(dnsException(err, bindingName, hostname));
-    else {
+    else if (hasObserver('dns')) {
       startPerf(req, kPerfHooksDnsLookupResolveContext, {
         type: 'dns',
         name: bindingName,
diff --git a/lib/internal/perf/observe.js b/lib/internal/perf/observe.js
index 4b226c2930c..c58aec7040f 100644
--- a/lib/internal/perf/observe.js
+++ b/lib/internal/perf/observe.js
@@ -460,27 +460,26 @@ function hasObserver(type) {
 
 
 function startPerf(target, key, context = {}) {
-  if (hasObserver(context.type)) {
-    target[key] = {
-      ...context,
-      startTime: now(),
-    };
-  }
+  target[key] = {
+    ...context,
+    startTime: now(),
+  };
 }
 
 function stopPerf(target, key, context = {}) {
   const ctx = target[key];
-  if (ctx && hasObserver(ctx.type)) {
-    const startTime = ctx.startTime;
-    const entry = new InternalPerformanceEntry(
-      ctx.name,
-      ctx.type,
-      startTime,
-      now() - startTime,
-      { ...ctx.detail, ...context.detail },
-    );
-    enqueue(entry);
+  if (!ctx) {
+    return;
   }
+  const startTime = ctx.startTime;
+  const entry = new InternalPerformanceEntry(
+    ctx.name,
+    ctx.type,
+    startTime,
+    now() - startTime,
+    { ...ctx.detail, ...context.detail },
+  );
+  enqueue(entry);
 }
 
 module.exports = {
diff --git a/lib/net.js b/lib/net.js
index c8c5b5447be..6f07cf6b9e8 100644
--- a/lib/net.js
+++ b/lib/net.js
@@ -133,6 +133,7 @@ const noop = () => {};
 
 const kPerfHooksNetConnectContext = Symbol('kPerfHooksNetConnectContext');
 const {
+  hasObserver,
   startPerf,
   stopPerf,
 } = require('internal/perf/observe');
@@ -999,7 +1000,7 @@ function internalConnect(
 
     const ex = exceptionWithHostPort(err, 'connect', address, port, details);
     self.destroy(ex);
-  } else if (addressType === 6 || addressType === 4) {
+  } else if ((addressType === 6 || addressType === 4) && hasObserver('net')) {
     startPerf(self, kPerfHooksNetConnectContext, { type: 'net', name: 'connect', detail: { host: address, port } });
   }
 }
@@ -1226,7 +1227,9 @@ function afterConnect(status, handle, req, readable, writable) {
     // this doesn't actually consume any bytes, because len=0.
     if (readable && !self.isPaused())
       self.read(0);
-    stopPerf(self, kPerfHooksNetConnectContext);
+    if (self[kPerfHooksNetConnectContext] && hasObserver('net')) {
+      stopPerf(self, kPerfHooksNetConnectContext);
+    }
   } else {
     self.connecting = false;
     let details;