Merge pull request #21250 from emberjs/copilot/improve-perf-script-reliability

make perf benchmark script more reliable

Author: NullVoxPopuli

bin/benchmark.mjs

@@ -17,7 +17,6 @@ Options:
 	 --force                  delete cached directories before running
      --reuse                  reuse existing apps and tarballs, if available (by default only the control app/tarball is reused)
      --no-headless            run Chrome without headless mode (opens visible browser windows)
-                              On macOS, benchmarks will run sequentially (one browser at a time)
 
 Notes:
 	- This script runs \`pnpm install\` and \`node ./bin/build-for-publishing.js\` in both repos.

bin/benchmark/run.mjs

@@ -6,7 +6,7 @@ import fs from 'fs-extra';
 
 import { getOrBuildControlTarball } from './control.mjs';
 import { buildExperimentTarball } from './experiment.mjs';
-import { run, prepareApp, sleep, startVitePreview, lsof } from './utils.mjs';
+import { run, prepareApp, startVitePreview, waitForServer } from './utils.mjs';
 
 const { ensureDir, remove } = fs;
 
@@ -112,12 +112,19 @@ export async function runBenchmark({ force = false, reuse = false, headless = tr
   ]);
 
   // These will error if the parts are occupied (--strict-port)
-  startVitePreview({ appDir: CONTROL_DIRS.app, port: DEFAULT_CONTROL_PORT });
-  startVitePreview({
+  const controlServer = startVitePreview({ appDir: CONTROL_DIRS.app, port: DEFAULT_CONTROL_PORT });
+  const experimentServer = startVitePreview({
     appDir: EXPERIMENT_DIRS.app,
     port: DEFAULT_EXPERIMENT_PORT,
   });
 
+  controlServer.catch((err) => {
+    console.error('Control server exited unexpectedly:', err.message);
+  });
+  experimentServer.catch((err) => {
+    console.error('Experiment server exited unexpectedly:', err.message);
+  });
+
   try {
     await bootAndRun({ headless });
   } finally {
@@ -132,20 +139,12 @@ async function bootAndRun({ headless = true } = {}) {
   const experimentUrl = `http://127.0.0.1:${DEFAULT_EXPERIMENT_PORT}`;
   const markersString = buildMarkersString(DEFAULT_MARKERS);
 
-  // give servers a moment to start
-  await sleep(5000);
-
-  /**
-   * We need to make sure both servers are running before starting the benchmark.
-   */
-  let controlLsof = await lsof(DEFAULT_CONTROL_PORT);
-  let experimentLsof = await lsof(DEFAULT_EXPERIMENT_PORT);
-
-  if (!controlLsof || !experimentLsof) {
-    throw new Error(
-      `One of the servers failed to start. Control server lsof:\n${controlLsof}\n\nExperiment server lsof:\n${experimentLsof}`
-    );
-  }
+  console.log('\n\tWaiting for servers to be ready...');
+  await Promise.all([
+    waitForServer(controlUrl, { timeout: 30_000 }),
+    waitForServer(experimentUrl, { timeout: 30_000 }),
+  ]);
+  console.log('\tBoth servers are ready.\n');
 
   const tracerbenchBin = join(REPO_ROOT, 'node_modules/tracerbench/bin/run');
 

bin/benchmark/utils.mjs

@@ -23,8 +23,37 @@ export function sleep(ms) {
   return new Promise((resolve) => setTimeout(resolve, ms));
 }
 
-export async function lsof(port) {
-  return execSync(`lsof -i :${port} -P -n`).toString;
+export function lsof(port) {
+  try {
+    return execSync(`lsof -i :${port} -P -n`).toString();
+  } catch {
+    return null;
+  }
+}
+
+/**
+ * Wait for a server to respond to HTTP requests.
+ * Polls with fetch until the server returns a response (any status),
+ * retrying on connection errors.
+ *
+ * @param {string} url - The URL to poll
+ * @param {object} [options]
+ * @param {number} [options.timeout=30000] - Max time to wait in ms
+ * @param {number} [options.interval=500] - Time between retries in ms
+ */
+export async function waitForServer(url, { timeout = 30_000, interval = 500 } = {}) {
+  const deadline = Date.now() + timeout;
+
+  while (Date.now() < deadline) {
+    try {
+      await fetch(url);
+      return;
+    } catch {
+      await sleep(interval);
+    }
+  }
+
+  throw new Error(`Server at ${url} did not respond within ${timeout}ms`);
 }
 
 export async function buildEmberSource(cwd) {

patches/@tracerbench__core@8.0.1.patch

@@ -58,3 +58,61 @@ index 5918e8f7665b3e796ef88283fc40c2b3286a564f..b9927ba8f639c0a9ecc70144362da439
      }
      return categories;
  }
+diff --git a/dist/metrics/extract-navigation-sample.js b/dist/metrics/extract-navigation-sample.js
+index 58a0330d018d00223d9cfe0cc353ee3903837df1..fa75690cd7c0f951a9106a9926caa288f6110143 100644
+--- a/dist/metrics/extract-navigation-sample.js
++++ b/dist/metrics/extract-navigation-sample.js
+@@ -40,6 +40,13 @@ function findPhases(events, markers) {
+                     }
+                 }
+                 else {
++                    // In headless Chrome on macOS, an initial navigationStart with an empty
++                    // documentLoaderURL may precede the real page navigation. Update
++                    // navigationStartArgs if we encounter a navigationStart with a real URL
++                    // so subsequent markers match the correct navigationId.
++                    if (event.name === 'navigationStart' && event.args?.data?.documentLoaderURL) {
++                        navigationStartArgs = event.args;
++                    }
+                     if (event.name === marker.start) {
+                         const { args } = event;
+                         // ensure this belongs to this navigation
+diff --git a/dist/util/inject-mark-observer.js b/dist/util/inject-mark-observer.js
+index 0444c9911387202e156d3901e16e1ff092399f75..f7385765297aff2ff6ece7c6164dd1d3de2b0047 100644
+--- a/dist/util/inject-mark-observer.js
++++ b/dist/util/inject-mark-observer.js
+@@ -54,8 +54,6 @@ function lcpObserver(variable, priorMarker) {
+ function markObserver(mark, variable) {
+     return `"use strict";
+     var ${variable} =
+-      self === top &&
+-      opener === null &&
+       new Promise((resolve) =>
+         new PerformanceObserver((records, observer) => {
+           if (records.getEntriesByName(${JSON.stringify(mark)}).length > 0) {
+@@ -66,7 +64,7 @@ function markObserver(mark, variable) {
+             });
+             observer.disconnect();
+           }
+-        }).observe({ type: 'mark' })
++        }).observe({ type: 'mark', buffered: true })
+       );`;
+ }
+ function navigationObserver(variable) {
+diff --git a/dist/util/run-trace.js b/dist/util/run-trace.js
+index 0f49cd51cf789117dd3ad9aea8754293f50a51ff..e428000306cc6553d094441ad5e17660962e06d6 100644
+--- a/dist/util/run-trace.js
++++ b/dist/util/run-trace.js
+@@ -10,7 +10,12 @@ async function runTrace(page, categories, raceCancellation, usingTracing, path)
+     page.on('Tracing.tracingComplete', complete);
+     try {
+         await performTrace(page, {
+-            categories: categories.join(','),
++            // Use traceConfig instead of deprecated categories string; use
++            // recordAsMuchAsPossible to avoid dropping events on ring-buffer overflow.
++            traceConfig: {
++                recordMode: 'recordAsMuchAsPossible',
++                includedCategories: categories,
++            },
+             transferMode: 'ReturnAsStream'
+         }, usingTracing, (0, race_cancellation_1.combineRaceCancellation)(raceEarlyComplete, raceCancellation));
+         stream = await waitForTraceStream(completed, raceCancellation);