Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

net,dns:move hasObserver out of perf function #43217

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
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 @@ -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