Skip to content

Commit ffe68ab

Browse files
fix: make perf benchmark script more reliable
- Fix lsof() bug: .toString (property ref) -> .toString() (method call) and wrap in try/catch for non-zero exit codes - Replace fixed sleep(5000) + broken lsof check with waitForServer() that polls with HTTP fetch until servers actually respond - Store startVitePreview child processes and add .catch() error handlers Co-authored-by: NullVoxPopuli <[email protected]> Agent-Logs-Url: https://github.com/emberjs/ember.js/sessions/abc30b6b-e3d1-44c5-988e-648096650173
1 parent 0531e62 commit ffe68ab

2 files changed

Lines changed: 50 additions & 19 deletions

File tree

bin/benchmark/run.mjs

Lines changed: 18 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import fs from 'fs-extra';
66

77
import { getOrBuildControlTarball } from './control.mjs';
88
import { buildExperimentTarball } from './experiment.mjs';
9-
import { run, prepareApp, sleep, startVitePreview, lsof } from './utils.mjs';
9+
import { run, prepareApp, startVitePreview, waitForServer } from './utils.mjs';
1010

1111
const { ensureDir, remove } = fs;
1212

@@ -112,12 +112,20 @@ export async function runBenchmark({ force = false, reuse = false, headless = tr
112112
]);
113113

114114
// These will error if the parts are occupied (--strict-port)
115-
startVitePreview({ appDir: CONTROL_DIRS.app, port: DEFAULT_CONTROL_PORT });
116-
startVitePreview({
115+
const controlServer = startVitePreview({ appDir: CONTROL_DIRS.app, port: DEFAULT_CONTROL_PORT });
116+
const experimentServer = startVitePreview({
117117
appDir: EXPERIMENT_DIRS.app,
118118
port: DEFAULT_EXPERIMENT_PORT,
119119
});
120120

121+
// Fail fast if either server process exits unexpectedly during startup
122+
controlServer.catch((err) => {
123+
console.error('Control server exited unexpectedly:', err.message);
124+
});
125+
experimentServer.catch((err) => {
126+
console.error('Experiment server exited unexpectedly:', err.message);
127+
});
128+
121129
try {
122130
await bootAndRun({ headless });
123131
} finally {
@@ -132,20 +140,13 @@ async function bootAndRun({ headless = true } = {}) {
132140
const experimentUrl = `http://127.0.0.1:${DEFAULT_EXPERIMENT_PORT}`;
133141
const markersString = buildMarkersString(DEFAULT_MARKERS);
134142

135-
// give servers a moment to start
136-
await sleep(5000);
137-
138-
/**
139-
* We need to make sure both servers are running before starting the benchmark.
140-
*/
141-
let controlLsof = await lsof(DEFAULT_CONTROL_PORT);
142-
let experimentLsof = await lsof(DEFAULT_EXPERIMENT_PORT);
143-
144-
if (!controlLsof || !experimentLsof) {
145-
throw new Error(
146-
`One of the servers failed to start. Control server lsof:\n${controlLsof}\n\nExperiment server lsof:\n${experimentLsof}`
147-
);
148-
}
143+
// Wait for both servers to be ready (HTTP-level check with retries)
144+
console.log('\n\tWaiting for servers to be ready...');
145+
await Promise.all([
146+
waitForServer(controlUrl, { timeout: 30_000 }),
147+
waitForServer(experimentUrl, { timeout: 30_000 }),
148+
]);
149+
console.log('\tBoth servers are ready.\n');
149150

150151
const tracerbenchBin = join(REPO_ROOT, 'node_modules/tracerbench/bin/run');
151152

bin/benchmark/utils.mjs

Lines changed: 32 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,38 @@ export function sleep(ms) {
2323
return new Promise((resolve) => setTimeout(resolve, ms));
2424
}
2525

26-
export async function lsof(port) {
27-
return execSync(`lsof -i :${port} -P -n`).toString;
26+
export function lsof(port) {
27+
try {
28+
return execSync(`lsof -i :${port} -P -n`).toString();
29+
} catch {
30+
return null;
31+
}
32+
}
33+
34+
/**
35+
* Wait for a server to respond to HTTP requests.
36+
* Polls with fetch until the server returns a response (any status),
37+
* retrying on connection errors.
38+
*
39+
* @param {string} url - The URL to poll
40+
* @param {object} [options]
41+
* @param {number} [options.timeout=30000] - Max time to wait in ms
42+
* @param {number} [options.interval=500] - Time between retries in ms
43+
*/
44+
export async function waitForServer(url, { timeout = 30_000, interval = 500 } = {}) {
45+
const deadline = Date.now() + timeout;
46+
47+
while (Date.now() < deadline) {
48+
try {
49+
await fetch(url);
50+
return; // any response means the server is up
51+
} catch {
52+
// connection refused / not ready yet — wait and retry
53+
await sleep(interval);
54+
}
55+
}
56+
57+
throw new Error(`Server at ${url} did not respond within ${timeout}ms`);
2858
}
2959

3060
export async function buildEmberSource(cwd) {

0 commit comments

Comments
 (0)