Skip to content

Commit

Permalink
net,dns: move hasObserver out of perf function
Browse files Browse the repository at this point in the history
move the hasObserver out of startPerf and stopPerf
to avoid generating useless objects when these are no observer

PR-URL: #43217
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Minwoo Jung <nodecorelab@gmail.com>
  • Loading branch information
theanarkh authored and danielleadams committed Jun 13, 2022
1 parent d558b3c commit 9803b82
Show file tree
Hide file tree
Showing 4 changed files with 79 additions and 52 deletions.
69 changes: 42 additions & 27 deletions lib/dns.js
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ const kPerfHooksDnsLookupServiceContext = Symbol('kPerfHooksDnsLookupServiceCont
const kPerfHooksDnsLookupResolveContext = Symbol('kPerfHooksDnsLookupResolveContext');

const {
hasObserver,
startPerf,
stopPerf,
} = require('internal/perf/observe');
Expand All @@ -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);
}
}


Expand All @@ -102,7 +105,9 @@ function onlookupall(err, addresses) {
}

this.callback(null, addresses);
stopPerf(this, kPerfHooksDnsLookupContext);
if (this[kPerfHooksDnsLookupContext] && hasObserver('dns')) {
stopPerf(this, kPerfHooksDnsLookupContext);
}
}


Expand Down Expand Up @@ -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;
}

Expand All @@ -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);
}
}


Expand All @@ -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;
}

Expand All @@ -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);
}
}
}

Expand All @@ -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 });
Expand Down
24 changes: 17 additions & 7 deletions lib/internal/dns/promises.js
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ const kPerfHooksDnsLookupServiceContext = Symbol('kPerfHooksDnsLookupServiceCont
const kPerfHooksDnsLookupResolveContext = Symbol('kPerfHooksDnsLookupResolveContext');

const {
hasObserver,
startPerf,
stopPerf,
} = require('internal/perf/observe');
Expand All @@ -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) {
Expand All @@ -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) {
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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) {
Expand All @@ -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',
Expand All @@ -196,6 +203,7 @@ function createLookupServicePromise(hostname, port) {
port
}
});
}
});
}

Expand Down Expand Up @@ -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) {
Expand All @@ -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,
Expand Down
31 changes: 15 additions & 16 deletions lib/internal/perf/observe.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
Expand Down
7 changes: 5 additions & 2 deletions lib/net.js
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,7 @@ const noop = () => {};

const kPerfHooksNetConnectContext = Symbol('kPerfHooksNetConnectContext');
const {
hasObserver,
startPerf,
stopPerf,
} = require('internal/perf/observe');
Expand Down Expand Up @@ -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 } });
}
}
Expand Down Expand Up @@ -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;
Expand Down

0 comments on commit 9803b82

Please sign in to comment.