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 @targos
theanarkh authored and targos committed Jul 31, 2022
1 parent 1431861 commit d766cfa
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
View file
Expand Up @@ -68,6 +68,7 @@ const kPerfHooksDnsLookupServiceContext = Symbol('kPerfHooksDnsLookupServiceCont
const kPerfHooksDnsLookupResolveContext = Symbol('kPerfHooksDnsLookupResolveContext');

const {
hasObserver,
startPerf,
stopPerf,
} = require('internal/perf/observe');
Expand All @@ -81,7 +82,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 @@ -100,7 +103,9 @@ function onlookupall(err, addresses) {
}

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


Expand Down Expand Up @@ -173,13 +178,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 @@ -192,7 +199,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 @@ -217,14 +226,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 @@ -241,7 +252,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 @@ -264,14 +277,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', { value: bindingName });
Expand Down
24 changes: 17 additions & 7 deletions lib/internal/dns/promises.js
View file
Expand Up @@ -43,6 +43,7 @@ const kPerfHooksDnsLookupServiceContext = Symbol('kPerfHooksDnsLookupServiceCont
const kPerfHooksDnsLookupResolveContext = Symbol('kPerfHooksDnsLookupResolveContext');

const {
hasObserver,
startPerf,
stopPerf,
} = require('internal/perf/observe');
Expand All @@ -55,7 +56,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 @@ -76,7 +79,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 @@ -107,7 +112,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 @@ -156,7 +161,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 @@ -173,7 +180,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 @@ -182,6 +189,7 @@ function createLookupServicePromise(hostname, port) {
port
}
});
}
});
}

Expand Down Expand Up @@ -209,7 +217,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 @@ -227,7 +237,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
View file
Expand Up @@ -463,27 +463,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
View file
Expand Up @@ -133,6 +133,7 @@ const noop = () => {};

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

Please sign in to comment.