Skip to content

Commit

Permalink
http: fixup perf regression
Browse files Browse the repository at this point in the history
Only call into hrtime if there's an observer

Also, fix up some previously missed changes from the original refactor

Signed-off-by: James M Snell <jasnell@gmail.com>
Refs: nodejs#37937
Refs: nodejs#37136
  • Loading branch information
jasnell committed Apr 6, 2021
1 parent 6986fa0 commit de05bb6
Show file tree
Hide file tree
Showing 4 changed files with 35 additions and 37 deletions.
12 changes: 9 additions & 3 deletions lib/_http_server.js
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,10 @@ const onResponseFinishChannel = dc.channel('http.server.response.finish');
const kServerResponse = Symbol('ServerResponse');
const kServerResponseStatistics = Symbol('ServerResponseStatistics');

const {
hasObserver,
} = require('internal/perf/observe');

const STATUS_CODES = {
100: 'Continue', // RFC 7231 6.2.1
101: 'Switching Protocols', // RFC 7231 6.2.2
Expand Down Expand Up @@ -194,9 +198,11 @@ function ServerResponse(req) {
this.shouldKeepAlive = false;
}

this[kServerResponseStatistics] = {
startTime: process.hrtime()
};
if (hasObserver('http')) {
this[kServerResponseStatistics] = {
startTime: process.hrtime()
};
}
}
ObjectSetPrototypeOf(ServerResponse.prototype, OutgoingMessage.prototype);
ObjectSetPrototypeOf(ServerResponse, OutgoingMessage);
Expand Down
9 changes: 7 additions & 2 deletions lib/internal/http.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,11 @@ const {
const { setUnrefTimeout } = require('internal/timers');

const { InternalPerformanceEntry } = require('internal/perf/perf');
const { enqueue } = require('internal/perf/observe');

const {
enqueue,
hasObserver,
} = require('internal/perf/observe');

let utcCache;

Expand All @@ -30,6 +34,7 @@ function resetCache() {
}

function emitStatistics(statistics) {
if (!hasObserver('http') || statistics == null) return;
const startTime = statistics.startTime;
const diff = process.hrtime(startTime);
const entry = new InternalPerformanceEntry(
Expand All @@ -46,5 +51,5 @@ module.exports = {
kOutHeaders: Symbol('kOutHeaders'),
kNeedDrain: Symbol('kNeedDrain'),
utcDate,
emitStatistics
emitStatistics,
};
50 changes: 18 additions & 32 deletions lib/internal/perf/observe.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ const {
const {
constants: {
NODE_PERFORMANCE_ENTRY_TYPE_GC,
NODE_PERFORMANCE_ENTRY_TYPE_FUNCTION,
NODE_PERFORMANCE_ENTRY_TYPE_HTTP2,
NODE_PERFORMANCE_ENTRY_TYPE_HTTP,
},
Expand Down Expand Up @@ -97,23 +96,18 @@ function queuePending() {
});
}

function getObserverType(type) {
switch (type) {
case 'gc': return NODE_PERFORMANCE_ENTRY_TYPE_GC;
case 'http2': return NODE_PERFORMANCE_ENTRY_TYPE_HTTP2;
case 'http': return NODE_PERFORMANCE_ENTRY_TYPE_HTTP;
}
}

function maybeDecrementObserverCounts(entryTypes) {
for (const type of entryTypes) {
let observerType;
switch (type) {
case 'gc':
observerType = NODE_PERFORMANCE_ENTRY_TYPE_GC;
break;
case 'function':
observerType = NODE_PERFORMANCE_ENTRY_TYPE_FUNCTION;
break;
case 'http2':
observerType = NODE_PERFORMANCE_ENTRY_TYPE_HTTP2;
break;
case 'http':
observerType = NODE_PERFORMANCE_ENTRY_TYPE_HTTP;
break;
}
const observerType = getObserverType(type);

if (observerType !== undefined) {
observerCounts[observerType]--;

Expand All @@ -126,24 +120,10 @@ function maybeDecrementObserverCounts(entryTypes) {
}

function maybeIncrementObserverCount(type) {
let observerType;
switch (type) {
case 'gc':
observerType = NODE_PERFORMANCE_ENTRY_TYPE_GC;
break;
case 'function':
observerType = NODE_PERFORMANCE_ENTRY_TYPE_FUNCTION;
break;
case 'http2':
observerType = NODE_PERFORMANCE_ENTRY_TYPE_HTTP2;
break;
case 'http':
observerType = NODE_PERFORMANCE_ENTRY_TYPE_HTTP;
break;
}
const observerType = getObserverType(type);

if (observerType !== undefined) {
observerCounts[observerType]++;

if (observerType === NODE_PERFORMANCE_ENTRY_TYPE_GC)
installGarbageCollectionTracking();
}
Expand Down Expand Up @@ -346,7 +326,13 @@ function observerCallback(name, type, startTime, duration, details) {

setupObservers(observerCallback);

function hasObserver(type) {
const observerType = getObserverType(type);
return observerCounts[observerType] > 0;
}

module.exports = {
PerformanceObserver,
enqueue,
hasObserver,
};
1 change: 1 addition & 0 deletions src/node_perf_common.h
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ extern uint64_t performance_v8_start;

#define NODE_PERFORMANCE_ENTRY_TYPES(V) \
V(GC, "gc") \
V(HTTP, "http") \
V(HTTP2, "http2")

enum PerformanceMilestone {
Expand Down

0 comments on commit de05bb6

Please sign in to comment.