Improve benchmark setup and rely on publishConfig

This commit has two related changes: 1. Clean up and document recent changes in the benchmark script. One of these changes is replacing ad-hoc rewrites of `package.json` files with a firm reliance on the `publishConfig` field, which should be reliable (since that's what we use when actually publishing). 2. Update all of the `package.json` files for published packages to have consistent `publishConfig` fields. This change also removed a bunch of historical cruft from the `package.json` files, which helped us to support environments during the ecosystem transition to package.json `exports` fields.
glimmerjs · Nov 3, 2024 · 406e17d · 406e17d
1 parent 57d9118
commit 406e17d
Show file tree

Hide file tree

Showing 28 changed files with 413 additions and 327 deletions.
diff --git a/.gitignore b/.gitignore
@@ -16,3 +16,4 @@ instrumentation.*.json
 .cache
 **/*.tgz
 tracerbench-results
+.rollup.cache
diff --git a/bin/published-packages.mts b/bin/published-packages.mts
@@ -0,0 +1,42 @@
+import { $ } from 'zx';
+import chalk from 'chalk';
+import { packages } from './packages.mjs';
+
+/*
+  Example JSON entry:
+
+  {
+    name: '@glimmer/validator',
+    version: '0.92.3',
+    path: '/home/ykatz/Code/Ember/glimmer-vm/packages/@glimmer/validator',
+    private: false,
+    dependencies: {
+      '@glimmer/env': [Object],
+      '@glimmer/global-context': [Object],
+      '@glimmer/interfaces': [Object],
+      '@glimmer/util': [Object]
+    },
+    devDependencies: {
+      '@glimmer-workspace/build-support': [Object],
+      '@glimmer/debug-util': [Object],
+      '@glimmer/local-debug-flags': [Object],
+      eslint: [Object],
+      publint: [Object],
+      rollup: [Object],
+      typescript: [Object]
+    }
+  }
+*/
+
+/**
+ * @typedef {} PackageEntry
+ */
+
+const entries = await packages('@glimmer');
+
+const quiet = process.argv.includes('--quiet') || process.argv.includes('-q');
+
+for (const entry of entries) {
+  console.log(entry.name);
+  if (!quiet) console.error(chalk.gray(`  ${entry.path}`));
+}
diff --git a/bin/setup-bench.mjs b/bin/setup-bench.mjs
@@ -1,10 +1,11 @@
 import 'zx/globals';
 import os from 'node:os';
 import { join } from 'node:path';
-import chalk from 'chalk';
+import { readFile, writeFile } from 'node:fs/promises';
 
 const ROOT = new URL('..', import.meta.url).pathname;
 $.verbose = true;
+
 const REUSE_CONTROL = !!process.env['REUSE_CONTROL'];
 
 /*
@@ -56,7 +57,6 @@ const markers = (process.env['MARKERS'] || appMarkers)
   .join(',');
 const fidelity = process.env['FIDELITY'] || '20';
 const throttleRate = process.env['THROTTLE'] || '2';
-const FORK_NAME = process.env['FORK_NAME'] || '';
 
 const tempDir = os.tmpdir();
 
@@ -84,81 +84,51 @@ if (!REUSE_CONTROL) {
 await $`rm -rf ${EXPERIMENT_DIR}`;
 await $`mkdir ${EXPERIMENT_DIR}`;
 
-const isMacOs = os.platform() === 'darwin';
-
+// Intentionally use the same folder for both experiment and control to make it easier to
+// make changes to the benchmark suite itself and compare the results.
 const BENCHMARK_FOLDER = join(pwd, benchmarkFolder);
 
 const CONTROL_PORT = 4020;
 const EXPERIMENT_PORT = 4021;
 const CONTROL_URL = `http://localhost:${CONTROL_PORT}`;
 const EXPERIMENT_URL = `http://localhost:${EXPERIMENT_PORT}`;
 
-// we can't do it in parallel on CI,
-
-if (!REUSE_CONTROL) {
-  // setup control
-  await within(async () => {
-    await $`git fetch origin`;
-    const mainRef = await $`git rev-parse origin/main`;
-    await cd(CONTROL_DIR);
-    await $`git clone ${join(ROOT, '.git')} .`;
-    await $`git reset --hard ${mainRef}`;
-    await $`rm -rf ./benchmark`;
-    await $`cp -r ${BENCHMARK_FOLDER} ./benchmark`;
-
-    console.info(`$ pnpm install --no-frozen-lockfile ${chalk.gray('[control]')}`);
-
-    await $`pwd`;
-    const result = await $`pnpm install`;
-    console.log(result);
-
-    console.info(`$ pnpm build ${chalk.gray('[control]')}`);
-
-    await $`pnpm build`;
-
-    if (isMacOs) {
-      await $`find ./packages -name 'package.json' -exec sed -i '' 's|"main": "index.ts",|"main": "./dist/prod/index.js","module": "./dist/prod/index.js",|g' {} \\;`;
-      await $`find ./packages -name 'package.json' -exec sed -i '' 's|"main": "./dist/index.js",|"main": "./dist/prod/index.js","module": "./dist/prod/index.js",|g' {} \\;`;
-      await $`find ./packages -name 'package.json' -exec sed -i '' 's|"import": "./dist/index.js"|"import": "./dist/prod/index.js"|g' {} \\;`;
-    } else {
-      await $`find ./packages -name 'package.json' -exec sed -i 's|"main": "index.ts",|"main": "./dist/prod/index.js","module": "./dist/prod/index.js",|g' {} \\;`;
-      await $`find ./packages -name 'package.json' -exec sed -i 's|"main": "./dist/index.js",|"main": "./dist/prod/index.js","module": "./dist/prod/index.js",|g' {} \\;`;
-      await $`find ./packages -name 'package.json' -exec sed -i 's|"import": "./dist/index.js"|"import": "./dist/prod/index.js"|g' {} \\;`;
-    }
+// make sure that the origin is up to date so we get the right control
+await $`git fetch origin`;
 
-    await cd(CONTROL_BENCH_DIR);
-    await $`pnpm vite build`;
-  });
-}
+// now we can get the ref of the control branch so we can check it out later
+const controlRef = (await $`git rev-parse origin/main`).stdout.trim();
 
-// setup experiment
-await within(async () => {
-  await cd(EXPERIMENT_DIR);
-  await $`git clone ${join(ROOT, '.git')} .`;
-  await $`git checkout --force ${experimentRef}`;
-  await $`rm -rf ./benchmark`;
-  await $`cp -r ${BENCHMARK_FOLDER} ./benchmark`;
+// we can't do it in parallel on CI,
 
-  console.info(`$ pnpm install --no-frozen-lockfile ${chalk.gray('[experiment]')}`);
-  const install = () => $`pnpm install --no-frozen-lockfile`.pipe(process.stderr);
-  await spinner(install);
-  console.info(`$ pnpm build ${chalk.gray('[experiment]')}`);
-  const build = () => $`pnpm build`.pipe(process.stderr);
-  await spinner(build);
-
-  if (isMacOs) {
-    await $`find ./packages -name 'package.json' -exec sed -i '' 's|"main": "index.ts",|"main": "./dist/prod/index.js","module": "./dist/prod/index.js",|g' {} \\;`;
-    await $`find ./packages -name 'package.json' -exec sed -i '' 's|"main": "./dist/index.js",|"main": "./dist/prod/index.js","module": "./dist/prod/index.js",|g' {} \\;`;
-    await $`find ./packages -name 'package.json' -exec sed -i '' 's|"import": "./dist/index.js"|"import": "./dist/prod/index.js"|g' {} \\;`;
-  } else {
-    await $`find ./packages -name 'package.json' -exec sed -i 's|"main": "index.ts",|"main": "./dist/prod/index.js","module": "./dist/prod/index.js",|g' {} \\;`;
-    await $`find ./packages -name 'package.json' -exec sed -i 's|"main": "./dist/index.js",|"main": "./dist/prod/index.js","module": "./dist/prod/index.js",|g' {} \\;`;
-    await $`find ./packages -name 'package.json' -exec sed -i 's|"import": "./dist/index.js"|"import": "./dist/prod/index.js"|g' {} \\;`;
+/**
+ * Rewrite all `package.json`s with a `publishConfig` field with the fields specified in
+ * `publishConfig`.
+ */
+async function rewritePackageJson() {
+  // limit to `@glimmer/*` packages
+  const packages = await $`find ./packages/@glimmer -name 'package.json'`;
+
+  for (const pkg of packages.stdout.trim().split('\n')) {
+    const packageJson = JSON.parse(await readFile(pkg, { encoding: 'utf8' }));
+    const publishConfig = packageJson['publishConfig'];
+
+    // assume that the presence of a `publishConfig` field means that the package is
+    // a published package and needs its package.json updated to behave like a published
+    // package in the benchmark environment.
+    if (publishConfig) {
+      const updatedPkg = { ...packageJson, ...publishConfig };
+
+      for (const [key, value] of Object.entries(publishConfig)) {
+        if (value === null) {
+          delete updatedPkg[key];
+        }
+      }
+
+      await writeFile(pkg, JSON.stringify(updatedPkg, null, 2), { encoding: 'utf8' });
+    }
   }
-
-  await cd(EXPERIMENT_BENCH_DIR);
-  await $`pnpm vite build`;
-});
+}
 
 console.info({
   control: controlBranchName,
@@ -167,6 +137,18 @@ console.info({
   CONTROL_DIR,
 });
 
+// setup experiment
+await within(async () => {
+  await buildRepo(EXPERIMENT_DIR, experimentRef);
+});
+
+if (!REUSE_CONTROL) {
+  // setup control
+  await within(async () => {
+    await buildRepo(CONTROL_DIR, controlRef);
+  });
+}
+
 // start build assets
 $`cd ${CONTROL_BENCH_DIR} && pnpm vite preview --port ${CONTROL_PORT}`;
 $`cd ${EXPERIMENT_BENCH_DIR} && pnpm vite preview --port ${EXPERIMENT_PORT}`;
@@ -194,3 +176,40 @@ try {
 }
 
 process.exit(0);
+
+/**
+ * @param {string} directory the directory to clone into
+ * @param {string} ref the ref to checkout
+ */
+async function buildRepo(directory, ref) {
+  // the benchmark directory is located in `packages/@glimmer/benchmark` in each of the
+  // experiment and control checkouts
+  const benchDir = join(directory, 'benchmark', 'benchmarks', 'krausest');
+
+  await cd(directory);
+
+  // write the `pwd` to the output to make it easier to debug if something goes wrong
+  await $`pwd`;
+
+  // clone the raw git repo for the experiment
+  await $`git clone ${join(ROOT, '.git')} .`;
+
+  // checkout the repo to the HEAD of the current branch
+  await $`git checkout --force ${ref}`;
+
+  // recreate the benchmark directory
+  await $`rm -rf ./benchmark`;
+  // intentionally use the same folder for both experiment and control
+  await $`cp -r ${BENCHMARK_FOLDER} ./benchmark`;
+
+  // `pnpm install` and build the repo
+  await $`pnpm install --no-frozen-lockfile`;
+  await $`pnpm build`;
+
+  // rewrite all `package.json`s to behave like published packages
+  await rewritePackageJson();
+
+  // build the benchmarks using vite
+  await cd(benchDir);
+  await $`pnpm vite build`;
+}
diff --git a/bin/tsconfig.json b/bin/tsconfig.json
@@ -2,18 +2,15 @@
   "compilerOptions": {
     "composite": true,
     "skipLibCheck": true,
-
     "baseUrl": ".",
     "allowJs": true,
     "checkJs": true,
     "outDir": "../ts-dist/bin",
-
     "target": "es2020",
     "module": "esnext",
     "moduleResolution": "bundler",
     "allowSyntheticDefaultImports": true,
     "verbatimModuleSyntax": true,
-
     "strict": true,
     "suppressImplicitAnyIndexErrors": false,
     "useDefineForClassFields": false,

diff --git a/packages/@glimmer-workspace/benchmark-env/package.json b/packages/@glimmer-workspace/benchmark-env/package.json
@@ -3,6 +3,7 @@
   "version": "0.84.3",
   "private": true,
   "repository": "https://github.com/glimmerjs/glimmer-vm/tree/main/packages/@glimmer-workspace/benchmark-env",
+  "type": "module",
   "main": "index.ts",
   "scripts": {
     "test:lint": "eslint .",

diff --git a/packages/@glimmer-workspace/integration-tests/test/chaos-rehydration-test.ts b/packages/@glimmer-workspace/integration-tests/test/chaos-rehydration-test.ts
@@ -89,11 +89,10 @@ abstract class AbstractChaosMonkeyTest extends RenderTest {
 
     let removedNodeDisplay: Nullable<string>;
     switch (nodeToRemove.nodeType) {
-      // eslint-disable-next-line @typescript-eslint/no-unsafe-enum-comparison
       case COMMENT_NODE:
         removedNodeDisplay = `<!--${nodeToRemove.nodeValue}-->`;
         break;
-      // eslint-disable-next-line @typescript-eslint/no-unsafe-enum-comparison
+
       case ELEMENT_NODE:
         removedNodeDisplay = castToBrowser(nodeToRemove, ['HTML', 'SVG']).outerHTML;
         break;

diff --git a/packages/@glimmer/compiler/package.json b/packages/@glimmer/compiler/package.json
@@ -4,29 +4,29 @@
   "license": "MIT",
   "repository": "https://github.com/glimmerjs/glimmer-vm/tree/main/packages/@glimmer/compiler",
   "type": "module",
-  "main": "index.ts",
-  "types": "index.ts",
-  "exports": {
-    "types": "./index.ts",
-    "development": "./index.ts",
-    "import": "./dist/index.js"
-  },
+  "exports": "./index.ts",
   "publishConfig": {
     "access": "public",
-    "types": "dist/dev/index.d.ts",
     "exports": {
       ".": {
-        "types": "./dist/dev/index.d.ts",
-        "development": {
-          "require": "./dist/dev/index.cjs",
-          "import": "./dist/dev/index.js"
+        "require": {
+          "development": {
+            "types": "./dist/dev/index.d.cts",
+            "default": "./dist/dev/index.cjs"
+          }
         },
-        "require": "./dist/dev/index.cjs",
-        "import": "./dist/dev/index.js"
+        "default": {
+          "development": {
+            "types": "./dist/dev/index.d.ts",
+            "default": "./dist/dev/index.js"
+          },
+          "default": {
+            "types": "./dist/prod/index.d.ts",
+            "default": "./dist/prod/index.js"
+          }
+        }
       }
-    },
-    "main": null,
-    "module": "dist/dev/index.js"
+    }
   },
   "files": [
     "dist"

diff --git a/packages/@glimmer/debug-util/package.json b/packages/@glimmer/debug-util/package.json
@@ -6,32 +6,9 @@
   "description": "Common utilities used in Glimmer with debug-specific behavior",
   "repository": "https://github.com/tildeio/glimmer/tree/main/packages/@glimmer/debug-util",
   "type": "module",
-  "main": "index.ts",
-  "types": "index.ts",
-  "publishConfig": {
-    "access": "public",
-    "types": "dist/dev/index.d.ts",
-    "exports": {
-      ".": {
-        "types": "./dist/dev/index.d.ts",
-        "development": {
-          "require": "./dist/dev/index.cjs",
-          "import": "./dist/dev/index.js"
-        },
-        "require": "./dist/dev/index.cjs",
-        "import": "./dist/prod/index.js"
-      }
-    },
-    "main": null,
-    "module": "dist/dev/index.js"
-  },
-  "files": [
-    "dist"
-  ],
+  "exports": "./index.ts",
   "scripts": {
-    "build": "rollup -c rollup.config.mjs",
     "test:lint": "eslint .",
-    "test:publint": "publint",
     "test:types": "tsc --noEmit -p ../tsconfig.json"
   },
   "dependencies": {

diff --git a/packages/@glimmer/debug/package.json b/packages/@glimmer/debug/package.json
@@ -7,29 +7,7 @@
   "sideEffects": false,
   "repository": "https://github.com/glimmerjs/glimmer-vm/tree/main/packages/@glimmer/debug",
   "type": "module",
-  "main": "index.ts",
-  "types": "index.ts",
-  "exports": {
-    "types": "./index.ts",
-    "development": "./index.ts"
-  },
-  "publishConfig": {
-    "access": "public",
-    "types": "dist/dev/index.d.ts",
-    "exports": {
-      ".": {
-        "types": "./dist/dev/index.d.ts",
-        "development": {
-          "import": "./dist/dev/index.js"
-        }
-      }
-    },
-    "main": null,
-    "module": "dist/dev/index.js"
-  },
-  "files": [
-    "dist"
-  ],
+  "exports": "./index.ts",
   "scripts": {
     "test:lint": "eslint .",
     "test:types": "tsc --noEmit -p ../tsconfig.json"