Skip to content

Commit 733d0f7

Browse files
committed
fix: address review feedback on the CPU benchmark harness
Settle in-flight CDP requests when the inspector socket closes or errors, and guard the message parse. A webapp that exited mid-run previously left every send() pending, so the bench hung until the suite timeout with nothing explaining why; a parse throw inside the listener surfaced as an uncaughtException and tore down the runner. send() now also rejects instead of hanging when the socket is already closed. Unref both event-loop-utilization interval timers so a throw between start and stop cannot keep a vitest worker alive. Make eventLoopUtilizationMonitor.enable() idempotent: a second call previously started another interval and overwrote the only stored cleanup callback, leaking the first. Validate --top in the profile analyzer. A non-numeric value produced NaN, and slice(0, NaN) silently printed empty tables that looked like an empty profile. Keep the release note behavioural, without the env var name.
1 parent ffc997d commit 733d0f7

5 files changed

Lines changed: 47 additions & 6 deletions

File tree

.server-changes/reduce-webapp-cpu-on-worker-routes.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,4 +3,4 @@ area: webapp
33
type: improvement
44
---
55

6-
Cut webapp CPU usage by about a quarter on the routes that workers call most, freeing headroom at the same request rate. Detailed event-loop blocking diagnostics are now off by default (set `EVENT_LOOP_MONITOR_ENABLED=1` to restore them); the event-loop utilization metric is unaffected.
6+
Cut webapp CPU usage by about a quarter on the routes that workers call most, freeing headroom at the same request rate. Detailed event-loop blocking traces are no longer recorded by default, because producing them was itself a large part of that cost.

apps/webapp/app/eventLoopMonitor.server.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,10 @@ export const eventLoopUtilizationMonitor = singleton("eventLoopUtilizationMonito
123123

124124
return {
125125
enable: () => {
126+
if (stop) {
127+
return;
128+
}
129+
126130
console.log("🥸 Initializing event loop utilization monitor");
127131

128132
stop = startEventLoopUtilizationMonitoring();

apps/webapp/test/bench/analyzeProfile.ts

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,8 +31,15 @@ function parseArgs(argv: string[]): {
3131

3232
for (let i = 0; i < argv.length; i++) {
3333
const arg = argv[i]!;
34-
if (arg === "--top") top = Number(argv[++i]);
35-
else if (arg === "--json") json = argv[++i];
34+
if (arg === "--top") {
35+
const raw = argv[++i];
36+
const parsed = Number(raw);
37+
if (!Number.isFinite(parsed) || parsed <= 0) {
38+
console.error(`--top expects a positive number, got "${raw ?? ""}"`);
39+
process.exit(1);
40+
}
41+
top = parsed;
42+
} else if (arg === "--json") json = argv[++i];
3643
else if (arg === "--root") root = resolve(argv[++i]!);
3744
else if (!arg.startsWith("--")) profilePath = arg;
3845
}

apps/webapp/test/bench/lib/cdp.ts

Lines changed: 29 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -68,17 +68,37 @@ class CdpSession {
6868
private nextId = 1;
6969
private pending = new Map<number, { resolve: (v: any) => void; reject: (e: Error) => void }>();
7070

71+
/**
72+
* Anything that ends the socket has to settle the in-flight requests. If the
73+
* profiled webapp exits mid-run, an unsettled `send()` would otherwise hang
74+
* until the suite-level timeout with nothing explaining why.
75+
*/
7176
private constructor(ws: WebSocket) {
7277
this.ws = ws;
7378
this.ws.on("message", (data) => {
74-
const msg = JSON.parse(data.toString()) as CdpMessage;
79+
let msg: CdpMessage;
80+
try {
81+
msg = JSON.parse(data.toString()) as CdpMessage;
82+
} catch {
83+
return;
84+
}
7585
if (msg.id === undefined) return;
7686
const waiter = this.pending.get(msg.id);
7787
if (!waiter) return;
7888
this.pending.delete(msg.id);
7989
if (msg.error) waiter.reject(new Error(`${msg.error.message} (${msg.error.code})`));
8090
else waiter.resolve(msg.result);
8191
});
92+
93+
const rejectAll = (reason: string) => {
94+
for (const waiter of this.pending.values()) {
95+
waiter.reject(new Error(reason));
96+
}
97+
this.pending.clear();
98+
};
99+
100+
this.ws.on("error", (err: Error) => rejectAll(`CDP socket error: ${err.message}`));
101+
this.ws.on("close", () => rejectAll("CDP socket closed before the response arrived"));
82102
}
83103

84104
/**
@@ -96,6 +116,10 @@ class CdpSession {
96116
}
97117

98118
send<T = any>(method: string, params: Record<string, unknown> = {}): Promise<T> {
119+
if (this.ws.readyState !== WebSocket.OPEN) {
120+
return Promise.reject(new Error(`CDP socket is not open, cannot send ${method}`));
121+
}
122+
99123
const id = this.nextId++;
100124
const promise = new Promise<T>((resolve, reject) => {
101125
this.pending.set(id, { resolve, reject });
@@ -155,13 +179,16 @@ export class WebappProfiler {
155179

156180
void this.evaluateElu();
157181

158-
this.eluTimer = setInterval(() => {
182+
const timer = setInterval(() => {
159183
void this.evaluateElu().then((utilization) => {
160184
if (utilization !== undefined) {
161185
this.eluSamples.push({ atMs: Date.now() - this.eluStartedAt, utilization });
162186
}
163187
});
164188
}, intervalMs);
189+
190+
timer.unref();
191+
this.eluTimer = timer;
165192
}
166193

167194
/**

internal-packages/run-engine/src/engine/bench/inspectorProfiler.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -97,12 +97,15 @@ export class InProcessProfiler {
9797
this.eluSamples = [];
9898
this.lastElu = performance.eventLoopUtilization();
9999

100-
this.eluTimer = setInterval(() => {
100+
const timer = setInterval(() => {
101101
const current = performance.eventLoopUtilization();
102102
const diff = performance.eventLoopUtilization(current, this.lastElu!);
103103
this.lastElu = current;
104104
this.eluSamples.push(Number.isFinite(diff.utilization) ? diff.utilization : 0);
105105
}, intervalMs);
106+
107+
timer.unref();
108+
this.eluTimer = timer;
106109
}
107110

108111
stopEluSampling(): EluStats {

0 commit comments

Comments
 (0)