Skip to content

Commit 85f8821

Browse files
theanarkhtheanarkh
authored
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: #43217 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Minwoo Jung <nodecorelab@gmail.com>
1 parent c977ad6 commit 85f8821

File tree

4 files changed

+79
-52
lines changed

4 files changed

+79
-52
lines changed

‎lib/dns.js‎

Lines changed: 42 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,7 @@ const kPerfHooksDnsLookupServiceContext = Symbol('kPerfHooksDnsLookupServiceCont
7070
const kPerfHooksDnsLookupResolveContext = Symbol('kPerfHooksDnsLookupResolveContext');
7171

7272
const {
73+
hasObserver,
7374
startPerf,
7475
stopPerf,
7576
} = require('internal/perf/observe');
@@ -83,7 +84,9 @@ function onlookup(err, addresses) {
8384
return this.callback(dnsException(err, 'getaddrinfo', this.hostname));
8485
}
8586
this.callback(null, addresses[0], this.family || isIP(addresses[0]));
86-
stopPerf(this, kPerfHooksDnsLookupContext);
87+
if (this[kPerfHooksDnsLookupContext] && hasObserver('dns')) {
88+
stopPerf(this, kPerfHooksDnsLookupContext);
89+
}
8790
}
8891

8992

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

104107
this.callback(null, addresses);
105-
stopPerf(this, kPerfHooksDnsLookupContext);
108+
if (this[kPerfHooksDnsLookupContext] && hasObserver('dns')) {
109+
stopPerf(this, kPerfHooksDnsLookupContext);
110+
}
106111
}
107112

108113

@@ -187,13 +192,15 @@ function lookup(hostname, options, callback) {
187192
process.nextTick(callback, dnsException(err, 'getaddrinfo', hostname));
188193
return {};
189194
}
190-
const detail = {
191-
hostname,
192-
family,
193-
hints,
194-
verbatim,
195-
};
196-
startPerf(req, kPerfHooksDnsLookupContext, { type: 'dns', name: 'lookup', detail });
195+
if (hasObserver('dns')) {
196+
const detail = {
197+
hostname,
198+
family,
199+
hints,
200+
verbatim,
201+
};
202+
startPerf(req, kPerfHooksDnsLookupContext, { type: 'dns', name: 'lookup', detail });
203+
}
197204
return req;
198205
}
199206

@@ -206,7 +213,9 @@ function onlookupservice(err, hostname, service) {
206213
return this.callback(dnsException(err, 'getnameinfo', this.hostname));
207214

208215
this.callback(null, hostname, service);
209-
stopPerf(this, kPerfHooksDnsLookupServiceContext);
216+
if (this[kPerfHooksDnsLookupServiceContext] && hasObserver('dns')) {
217+
stopPerf(this, kPerfHooksDnsLookupServiceContext);
218+
}
210219
}
211220

212221

@@ -231,14 +240,16 @@ function lookupService(address, port, callback) {
231240

232241
const err = cares.getnameinfo(req, address, port);
233242
if (err) throw dnsException(err, 'getnameinfo', address);
234-
startPerf(req, kPerfHooksDnsLookupServiceContext, {
235-
type: 'dns',
236-
name: 'lookupService',
237-
detail: {
238-
host: address,
239-
port
240-
}
241-
});
243+
if (hasObserver('dns')) {
244+
startPerf(req, kPerfHooksDnsLookupServiceContext, {
245+
type: 'dns',
246+
name: 'lookupService',
247+
detail: {
248+
host: address,
249+
port,
250+
},
251+
});
252+
}
242253
return req;
243254
}
244255

@@ -255,7 +266,9 @@ function onresolve(err, result, ttls) {
255266
this.callback(dnsException(err, this.bindingName, this.hostname));
256267
else {
257268
this.callback(null, result);
258-
stopPerf(this, kPerfHooksDnsLookupResolveContext);
269+
if (this[kPerfHooksDnsLookupResolveContext] && hasObserver('dns')) {
270+
stopPerf(this, kPerfHooksDnsLookupResolveContext);
271+
}
259272
}
260273
}
261274

@@ -278,14 +291,16 @@ function resolver(bindingName) {
278291
req.ttl = !!(options && options.ttl);
279292
const err = this._handle[bindingName](req, toASCII(name));
280293
if (err) throw dnsException(err, bindingName, name);
281-
startPerf(req, kPerfHooksDnsLookupResolveContext, {
282-
type: 'dns',
283-
name: bindingName,
284-
detail: {
285-
host: name,
286-
ttl: req.ttl
287-
}
288-
});
294+
if (hasObserver('dns')) {
295+
startPerf(req, kPerfHooksDnsLookupResolveContext, {
296+
type: 'dns',
297+
name: bindingName,
298+
detail: {
299+
host: name,
300+
ttl: req.ttl,
301+
},
302+
});
303+
}
289304
return req;
290305
}
291306
ObjectDefineProperty(query, 'name', { __proto__: null, value: bindingName });

‎lib/internal/dns/promises.js‎

Lines changed: 17 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ const kPerfHooksDnsLookupServiceContext = Symbol('kPerfHooksDnsLookupServiceCont
4646
const kPerfHooksDnsLookupResolveContext = Symbol('kPerfHooksDnsLookupResolveContext');
4747

4848
const {
49+
hasObserver,
4950
startPerf,
5051
stopPerf,
5152
} = require('internal/perf/observe');
@@ -58,7 +59,9 @@ function onlookup(err, addresses) {
5859

5960
const family = this.family || isIP(addresses[0]);
6061
this.resolve({ address: addresses[0], family });
61-
stopPerf(this, kPerfHooksDnsLookupContext);
62+
if (this[kPerfHooksDnsLookupContext] && hasObserver('dns')) {
63+
stopPerf(this, kPerfHooksDnsLookupContext);
64+
}
6265
}
6366

6467
function onlookupall(err, addresses) {
@@ -79,7 +82,9 @@ function onlookupall(err, addresses) {
7982
}
8083

8184
this.resolve(addresses);
82-
stopPerf(this, kPerfHooksDnsLookupContext);
85+
if (this[kPerfHooksDnsLookupContext] && hasObserver('dns')) {
86+
stopPerf(this, kPerfHooksDnsLookupContext);
87+
}
8388
}
8489

8590
function createLookupPromise(family, hostname, all, hints, verbatim) {
@@ -110,7 +115,7 @@ function createLookupPromise(family, hostname, all, hints, verbatim) {
110115

111116
if (err) {
112117
reject(dnsException(err, 'getaddrinfo', hostname));
113-
} else {
118+
} else if (hasObserver('dns')) {
114119
const detail = {
115120
hostname,
116121
family,
@@ -170,7 +175,9 @@ function onlookupservice(err, hostname, service) {
170175
}
171176

172177
this.resolve({ hostname, service });
173-
stopPerf(this, kPerfHooksDnsLookupServiceContext);
178+
if (this[kPerfHooksDnsLookupServiceContext] && hasObserver('dns')) {
179+
stopPerf(this, kPerfHooksDnsLookupServiceContext);
180+
}
174181
}
175182

176183
function createLookupServicePromise(hostname, port) {
@@ -187,7 +194,7 @@ function createLookupServicePromise(hostname, port) {
187194

188195
if (err)
189196
reject(dnsException(err, 'getnameinfo', hostname));
190-
else
197+
else if (hasObserver('dns')) {
191198
startPerf(req, kPerfHooksDnsLookupServiceContext, {
192199
type: 'dns',
193200
name: 'lookupService',
@@ -196,6 +203,7 @@ function createLookupServicePromise(hostname, port) {
196203
port
197204
}
198205
});
206+
}
199207
});
200208
}
201209

@@ -223,7 +231,9 @@ function onresolve(err, result, ttls) {
223231
result, (address, index) => ({ address, ttl: ttls[index] }));
224232

225233
this.resolve(result);
226-
stopPerf(this, kPerfHooksDnsLookupResolveContext);
234+
if (this[kPerfHooksDnsLookupResolveContext] && hasObserver('dns')) {
235+
stopPerf(this, kPerfHooksDnsLookupResolveContext);
236+
}
227237
}
228238

229239
function createResolverPromise(resolver, bindingName, hostname, ttl) {
@@ -241,7 +251,7 @@ function createResolverPromise(resolver, bindingName, hostname, ttl) {
241251

242252
if (err)
243253
reject(dnsException(err, bindingName, hostname));
244-
else {
254+
else if (hasObserver('dns')) {
245255
startPerf(req, kPerfHooksDnsLookupResolveContext, {
246256
type: 'dns',
247257
name: bindingName,

‎lib/internal/perf/observe.js‎

Lines changed: 15 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -460,27 +460,26 @@ function hasObserver(type) {
460460

461461

462462
function startPerf(target, key, context = {}) {
463-
if (hasObserver(context.type)) {
464-
target[key] = {
465-
...context,
466-
startTime: now(),
467-
};
468-
}
463+
target[key] = {
464+
...context,
465+
startTime: now(),
466+
};
469467
}
470468

471469
function stopPerf(target, key, context = {}) {
472470
const ctx = target[key];
473-
if (ctx && hasObserver(ctx.type)) {
474-
const startTime = ctx.startTime;
475-
const entry = new InternalPerformanceEntry(
476-
ctx.name,
477-
ctx.type,
478-
startTime,
479-
now() - startTime,
480-
{ ...ctx.detail, ...context.detail },
481-
);
482-
enqueue(entry);
471+
if (!ctx) {
472+
return;
483473
}
474+
const startTime = ctx.startTime;
475+
const entry = new InternalPerformanceEntry(
476+
ctx.name,
477+
ctx.type,
478+
startTime,
479+
now() - startTime,
480+
{ ...ctx.detail, ...context.detail },
481+
);
482+
enqueue(entry);
484483
}
485484

486485
module.exports = {

‎lib/net.js‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -133,6 +133,7 @@ const noop = () => {};
133133

134134
const kPerfHooksNetConnectContext = Symbol('kPerfHooksNetConnectContext');
135135
const {
136+
hasObserver,
136137
startPerf,
137138
stopPerf,
138139
} = require('internal/perf/observe');
@@ -999,7 +1000,7 @@ function internalConnect(
9991000

10001001
const ex = exceptionWithHostPort(err, 'connect', address, port, details);
10011002
self.destroy(ex);
1002-
} else if (addressType === 6 || addressType === 4) {
1003+
} else if ((addressType === 6 || addressType === 4) && hasObserver('net')) {
10031004
startPerf(self, kPerfHooksNetConnectContext, { type: 'net', name: 'connect', detail: { host: address, port } });
10041005
}
10051006
}
@@ -1226,7 +1227,9 @@ function afterConnect(status, handle, req, readable, writable) {
12261227
// this doesn't actually consume any bytes, because len=0.
12271228
if (readable && !self.isPaused())
12281229
self.read(0);
1229-
stopPerf(self, kPerfHooksNetConnectContext);
1230+
if (self[kPerfHooksNetConnectContext] && hasObserver('net')) {
1231+
stopPerf(self, kPerfHooksNetConnectContext);
1232+
}
12301233
} else {
12311234
self.connecting = false;
12321235
let details;

0 commit comments

Comments
 (0)