Skip to content

Commit 6783347

Browse files
perf(ext/node): skip node:http perf timing without observers (#34409)
## Summary Avoids unconditional `performance.now()` work on the node:http server request hot path when there are no Node `PerformanceObserver`s interested in `entryType: "http"`. The previous implementation timestamped every incoming server request, then discovered at finish time whether any observer wanted an `HttpRequest` entry. This patch mirrors Node's shape more closely: Node gates `startPerf()` in `lib/_http_server.js` behind `hasObserver('http')`, and gates `_finish()` behind both stored perf state and `hasObserver('http')`. ## Node compatibility Checked upstream Node main: - `lib/_http_server.js`: `ServerResponse` calls `startPerf(...)` only when `hasObserver('http')` is true. - `lib/_http_server.js`: `_finish()` calls `stopPerf(...)` only when response perf state exists and `hasObserver('http')` is still true. - `lib/internal/perf/observe.js`: `hasObserver(type)` reads the internal observer count for the entry type. Behavior spot-checks against Node v25.9.0: - Observer attached before a request sees an `HttpRequest` entry with request and response detail. - Observer attached after request start but before response finish does not receive a retroactive server entry. - Observer disconnected before response finish does not receive the entry. ## Validation Correctness: ```sh DENO_TEST_UTIL_DENO_EXE=/home/bot/work/stuff/shared/a3f6b36b-f67/workers/313cb9ff-3b9/perf-http-observer-validation-20260527T014052Z/deno-candidate cargo test -p unit_node_tests --test unit_node -- perf_hooks_test ``` Result: passed, 1 file-test (`tests/unit_node/perf_hooks_test.ts`). Benchmark command, execution target `bigboi`: ```sh RUN_DIR=/home/bot/work/stuff/shared/a3f6b36b-f67/workers/c68aca61-79c/perf-http-observer-rerun-20260527T055454Z \ BASELINE=/home/bot/work/stuff/shared/a3f6b36b-f67/workers/313cb9ff-3b9/perf-http-observer-validation-20260527T014052Z/deno-baseline \ CANDIDATE=/home/bot/work/stuff/shared/a3f6b36b-f67/workers/313cb9ff-3b9/perf-http-observer-validation-20260527T014052Z/deno-candidate \ SERVER=$RUN_DIR/server_get.mjs \ SAMPLES=8 DURATION=8s WARMUP=3s CONNECTIONS=128 SERVER_CPU=2 CLIENT_CPUS=3-5 \ $RUN_DIR/run_get_bench.sh ``` Fresh 8-sample rerun: - Baseline: 10,987.68 req/s mean, median 10,813.26, CV 7.72%. - Candidate: 11,482.77 req/s mean, median 11,406.40, CV 8.38%. - Paired delta: +495.10 req/s, +5.04% mean; 6/8 pairs positive. - Confidence: directional improvement, but noisy; the 95% interval on percent delta crosses zero. Earlier same-target 20-sample validation using the same release-lite binaries: - Baseline: 14,909.34 req/s mean, CV 10.54%. - Candidate: 15,419.35 req/s mean, CV 6.21%. - Paired delta: +510.01 req/s, +4.07% mean; 13/20 pairs positive. - 95% CI: delta req/s [-65.93, 1085.96], percent delta [0.22%, 7.92%]. - Confidence: moderate; noisy host, but the larger paired run supports a small real improvement. Co-authored-by: Nathan Whitaker <nathan@deno.com>
1 parent 2e016f4 commit 6783347

3 files changed

Lines changed: 105 additions & 5 deletions

File tree

ext/node/polyfills/_http_server.js

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -88,9 +88,10 @@ const {
8888
enterAsyncResource,
8989
exitAsyncResource,
9090
} = core.loadExtScript("ext:deno_node/internal/async_hooks.ts");
91-
const { enqueueNodePerformanceEntry } = core.loadExtScript(
92-
"ext:deno_node/perf_hooks.js",
93-
);
91+
const { enqueueNodePerformanceEntry, hasNodeObserverForType } = core
92+
.loadExtScript(
93+
"ext:deno_node/perf_hooks.js",
94+
);
9495
const {
9596
otelState,
9697
builtinTracer,
@@ -824,7 +825,9 @@ function parserOnIncoming(server, socket, state, req, keepAlive) {
824825
}
825826

826827
state.incoming.push(req);
827-
req[kPerfStartTime] = performance.now();
828+
if (hasNodeObserverForType("http")) {
829+
req[kPerfStartTime] = performance.now();
830+
}
828831

829832
if (!socket._paused) {
830833
const ws = socket._writableState;
@@ -1009,7 +1012,7 @@ function resOnFinish(req, res, socket, state, server) {
10091012

10101013
// Emit HttpRequest perf entry
10111014
const perfStartTime = req[kPerfStartTime];
1012-
if (perfStartTime !== undefined) {
1015+
if (perfStartTime !== undefined && hasNodeObserverForType("http")) {
10131016
enqueueNodePerformanceEntry({
10141017
name: "HttpRequest",
10151018
entryType: "http",

ext/node/polyfills/perf_hooks.js

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -160,6 +160,15 @@ function enqueueNodePerformanceEntry(entry) {
160160
}
161161
}
162162

163+
function hasNodeObserverForType(entryType) {
164+
for (let i = 0; i < nodeObservers.length; i++) {
165+
if (nodeObservers[i][_nodeTypes].includes(entryType)) {
166+
return true;
167+
}
168+
}
169+
return false;
170+
}
171+
163172
const eventLoopUtilization = () => {
164173
// TODO(@marvinhagemeister): Return actual non-stubbed values
165174
return { idle: 0, active: 0, utilization: 0 };
@@ -609,6 +618,7 @@ return {
609618
createHistogram,
610619
enqueueNodePerformanceEntry,
611620
eventLoopUtilization,
621+
hasNodeObserverForType,
612622
monitorEventLoopDelay,
613623
performance,
614624
PerformanceEntry,

tests/unit_node/perf_hooks_test.ts

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import {
55
PerformanceEntry,
66
PerformanceObserver,
77
} from "node:perf_hooks";
8+
import http from "node:http";
89
import { assert, assertEquals, assertThrows } from "@std/assert";
910

1011
// Basic performance API tests removed - covered by Node compat tests:
@@ -135,3 +136,89 @@ Deno.test("[perf_hooks]: PerformanceObserver takeRecords", () => {
135136
observer.disconnect();
136137
performance.clearMarks();
137138
});
139+
140+
Deno.test("[perf_hooks]: PerformanceObserver observes node:http server entries", async () => {
141+
const server = http.createServer((_req, res) => {
142+
res.end("ok");
143+
});
144+
await new Promise<void>((resolve) => server.listen(0, "127.0.0.1", resolve));
145+
146+
let timer: ReturnType<typeof setTimeout> | undefined;
147+
let observer: PerformanceObserver | undefined;
148+
const entryPromise = new Promise<PerformanceEntry>((resolve, reject) => {
149+
timer = setTimeout(
150+
() => reject(new Error("Timed out waiting for HTTP performance entry")),
151+
1000,
152+
);
153+
observer = new PerformanceObserver((list) => {
154+
const entry = list.getEntries().find((entry) =>
155+
entry.entryType === "http"
156+
);
157+
if (entry) {
158+
clearTimeout(timer);
159+
resolve(entry);
160+
}
161+
});
162+
observer.observe({ entryTypes: ["http"] });
163+
});
164+
165+
try {
166+
const address = server.address();
167+
if (!address || typeof address !== "object") {
168+
throw new Error("Server did not listen on a TCP address");
169+
}
170+
const response = await fetch(`http://127.0.0.1:${address.port}/observed`);
171+
assertEquals(await response.text(), "ok");
172+
173+
const entry = await entryPromise;
174+
assertEquals(entry.name, "HttpRequest");
175+
assertEquals(entry.entryType, "http");
176+
assert(entry.duration >= 0);
177+
} finally {
178+
if (timer !== undefined) clearTimeout(timer);
179+
observer?.disconnect();
180+
await new Promise<void>((resolve, reject) =>
181+
server.close((err) => err ? reject(err) : resolve())
182+
);
183+
}
184+
});
185+
186+
Deno.test("[perf_hooks]: node:http server entries are not retroactive", async () => {
187+
let finishResponse: (() => void) | undefined;
188+
let resolveRequestStarted = () => {};
189+
const requestStarted = new Promise<void>((resolve) => {
190+
resolveRequestStarted = resolve;
191+
});
192+
const server = http.createServer((_req, res) => {
193+
finishResponse = () => res.end("ok");
194+
resolveRequestStarted();
195+
});
196+
await new Promise<void>((resolve) => server.listen(0, "127.0.0.1", resolve));
197+
198+
const entries: PerformanceEntry[] = [];
199+
const observer = new PerformanceObserver((list) => {
200+
entries.push(...list.getEntries());
201+
});
202+
203+
try {
204+
const address = server.address();
205+
if (!address || typeof address !== "object") {
206+
throw new Error("Server did not listen on a TCP address");
207+
}
208+
const responsePromise = fetch(`http://127.0.0.1:${address.port}/late`);
209+
await requestStarted;
210+
211+
observer.observe({ entryTypes: ["http"] });
212+
finishResponse?.();
213+
const response = await responsePromise;
214+
assertEquals(await response.text(), "ok");
215+
await new Promise((resolve) => setTimeout(resolve, 50));
216+
217+
assertEquals(entries.length, 0);
218+
} finally {
219+
observer.disconnect();
220+
await new Promise<void>((resolve, reject) =>
221+
server.close((err) => err ? reject(err) : resolve())
222+
);
223+
}
224+
});

0 commit comments

Comments
 (0)