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
  • Loading branch information
theanarkh committed May 31, 2022
1 parent 122c377 commit 896bf69
Show file tree
Hide file tree
Showing 4 changed files with 83 additions and 56 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', { value: bindingName });
Expand Down
32 changes: 21 additions & 11 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,15 +194,16 @@ function createLookupServicePromise(hostname, port) {

if (err)
reject(dnsException(err, 'getnameinfo', hostname));
else
else if (hasObserver('dns')) {
startPerf(req, kPerfHooksDnsLookupServiceContext, {
type: 'dns',
name: 'lookupService',
detail: {
host: 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,14 +251,14 @@ 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,
detail: {
host: hostname,
ttl
}
ttl,
},
});
}
});
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 @@ -992,7 +993,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 @@ -1219,7 +1220,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 896bf69

Please sign in to comment.