Skip to content

Commit 57e59c4

Browse files
wycatswycats
authored
Improve benchmark setup, consistently rely on publishConfig (#1648)
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.
1 parent 57d9118 commit 57e59c4

File tree

29 files changed

+413
-329
lines changed

29 files changed

+413
-329
lines changed

‎.github/workflows/perf.yml

-2
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,6 @@ concurrency:
1212
cancel-in-progress: true
1313

1414
env:
15-
EXPERIMENT_BRANCH_NAME: ${{ github.head_ref || github.ref_name }}
16-
CONTROL_BRANCH_NAME: 'main'
1715
FIDELITY: 100
1816
THROTTLE: 4
1917
FORK_NAME: ${{ github.event.pull_request.head.repo.full_name }}

‎.gitignore

+1
Original file line numberDiff line numberDiff line change
@@ -16,3 +16,4 @@ instrumentation.*.json
1616
.cache
1717
**/*.tgz
1818
tracerbench-results
19+
.rollup.cache

‎bin/published-packages.mts

+42
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
import { $ } from 'zx';
2+
import chalk from 'chalk';
3+
import { packages } from './packages.mjs';
4+
5+
/*
6+
Example JSON entry:
7+
8+
{
9+
name: '@glimmer/validator',
10+
version: '0.92.3',
11+
path: '/home/ykatz/Code/Ember/glimmer-vm/packages/@glimmer/validator',
12+
private: false,
13+
dependencies: {
14+
'@glimmer/env': [Object],
15+
'@glimmer/global-context': [Object],
16+
'@glimmer/interfaces': [Object],
17+
'@glimmer/util': [Object]
18+
},
19+
devDependencies: {
20+
'@glimmer-workspace/build-support': [Object],
21+
'@glimmer/debug-util': [Object],
22+
'@glimmer/local-debug-flags': [Object],
23+
eslint: [Object],
24+
publint: [Object],
25+
rollup: [Object],
26+
typescript: [Object]
27+
}
28+
}
29+
*/
30+
31+
/**
32+
* @typedef {} PackageEntry
33+
*/
34+
35+
const entries = await packages('@glimmer');
36+
37+
const quiet = process.argv.includes('--quiet') || process.argv.includes('-q');
38+
39+
for (const entry of entries) {
40+
console.log(entry.name);
41+
if (!quiet) console.error(chalk.gray(` ${entry.path}`));
42+
}

‎bin/setup-bench.mjs

+85-66
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,11 @@
11
import 'zx/globals';
22
import os from 'node:os';
33
import { join } from 'node:path';
4-
import chalk from 'chalk';
4+
import { readFile, writeFile } from 'node:fs/promises';
55

66
const ROOT = new URL('..', import.meta.url).pathname;
77
$.verbose = true;
8+
89
const REUSE_CONTROL = !!process.env['REUSE_CONTROL'];
910

1011
/*
@@ -56,7 +57,6 @@ const markers = (process.env['MARKERS'] || appMarkers)
5657
.join(',');
5758
const fidelity = process.env['FIDELITY'] || '20';
5859
const throttleRate = process.env['THROTTLE'] || '2';
59-
const FORK_NAME = process.env['FORK_NAME'] || '';
6060

6161
const tempDir = os.tmpdir();
6262

@@ -84,81 +84,51 @@ if (!REUSE_CONTROL) {
8484
await $`rm -rf ${EXPERIMENT_DIR}`;
8585
await $`mkdir ${EXPERIMENT_DIR}`;
8686

87-
const isMacOs = os.platform() === 'darwin';
88-
87+
// Intentionally use the same folder for both experiment and control to make it easier to
88+
// make changes to the benchmark suite itself and compare the results.
8989
const BENCHMARK_FOLDER = join(pwd, benchmarkFolder);
9090

9191
const CONTROL_PORT = 4020;
9292
const EXPERIMENT_PORT = 4021;
9393
const CONTROL_URL = `http://localhost:${CONTROL_PORT}`;
9494
const EXPERIMENT_URL = `http://localhost:${EXPERIMENT_PORT}`;
9595

96-
// we can't do it in parallel on CI,
97-
98-
if (!REUSE_CONTROL) {
99-
// setup control
100-
await within(async () => {
101-
await $`git fetch origin`;
102-
const mainRef = await $`git rev-parse origin/main`;
103-
await cd(CONTROL_DIR);
104-
await $`git clone ${join(ROOT, '.git')} .`;
105-
await $`git reset --hard ${mainRef}`;
106-
await $`rm -rf ./benchmark`;
107-
await $`cp -r ${BENCHMARK_FOLDER} ./benchmark`;
108-
109-
console.info(`$ pnpm install --no-frozen-lockfile ${chalk.gray('[control]')}`);
110-
111-
await $`pwd`;
112-
const result = await $`pnpm install`;
113-
console.log(result);
114-
115-
console.info(`$ pnpm build ${chalk.gray('[control]')}`);
116-
117-
await $`pnpm build`;
118-
119-
if (isMacOs) {
120-
await $`find ./packages -name 'package.json' -exec sed -i '' 's|"main": "index.ts",|"main": "./dist/prod/index.js","module": "./dist/prod/index.js",|g' {} \\;`;
121-
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' {} \\;`;
122-
await $`find ./packages -name 'package.json' -exec sed -i '' 's|"import": "./dist/index.js"|"import": "./dist/prod/index.js"|g' {} \\;`;
123-
} else {
124-
await $`find ./packages -name 'package.json' -exec sed -i 's|"main": "index.ts",|"main": "./dist/prod/index.js","module": "./dist/prod/index.js",|g' {} \\;`;
125-
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' {} \\;`;
126-
await $`find ./packages -name 'package.json' -exec sed -i 's|"import": "./dist/index.js"|"import": "./dist/prod/index.js"|g' {} \\;`;
127-
}
96+
// make sure that the origin is up to date so we get the right control
97+
await $`git fetch origin`;
12898

129-
await cd(CONTROL_BENCH_DIR);
130-
await $`pnpm vite build`;
131-
});
132-
}
99+
// now we can get the ref of the control branch so we can check it out later
100+
const controlRef = (await $`git rev-parse origin/main`).stdout.trim();
133101

134-
// setup experiment
135-
await within(async () => {
136-
await cd(EXPERIMENT_DIR);
137-
await $`git clone ${join(ROOT, '.git')} .`;
138-
await $`git checkout --force ${experimentRef}`;
139-
await $`rm -rf ./benchmark`;
140-
await $`cp -r ${BENCHMARK_FOLDER} ./benchmark`;
102+
// we can't do it in parallel on CI,
141103

142-
console.info(`$ pnpm install --no-frozen-lockfile ${chalk.gray('[experiment]')}`);
143-
const install = () => $`pnpm install --no-frozen-lockfile`.pipe(process.stderr);
144-
await spinner(install);
145-
console.info(`$ pnpm build ${chalk.gray('[experiment]')}`);
146-
const build = () => $`pnpm build`.pipe(process.stderr);
147-
await spinner(build);
148-
149-
if (isMacOs) {
150-
await $`find ./packages -name 'package.json' -exec sed -i '' 's|"main": "index.ts",|"main": "./dist/prod/index.js","module": "./dist/prod/index.js",|g' {} \\;`;
151-
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' {} \\;`;
152-
await $`find ./packages -name 'package.json' -exec sed -i '' 's|"import": "./dist/index.js"|"import": "./dist/prod/index.js"|g' {} \\;`;
153-
} else {
154-
await $`find ./packages -name 'package.json' -exec sed -i 's|"main": "index.ts",|"main": "./dist/prod/index.js","module": "./dist/prod/index.js",|g' {} \\;`;
155-
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' {} \\;`;
156-
await $`find ./packages -name 'package.json' -exec sed -i 's|"import": "./dist/index.js"|"import": "./dist/prod/index.js"|g' {} \\;`;
104+
/**
105+
* Rewrite all `package.json`s with a `publishConfig` field with the fields specified in
106+
* `publishConfig`.
107+
*/
108+
async function rewritePackageJson() {
109+
// limit to `@glimmer/*` packages
110+
const packages = await $`find ./packages/@glimmer -name 'package.json'`;
111+
112+
for (const pkg of packages.stdout.trim().split('\n')) {
113+
const packageJson = JSON.parse(await readFile(pkg, { encoding: 'utf8' }));
114+
const publishConfig = packageJson['publishConfig'];
115+
116+
// assume that the presence of a `publishConfig` field means that the package is
117+
// a published package and needs its package.json updated to behave like a published
118+
// package in the benchmark environment.
119+
if (publishConfig) {
120+
const updatedPkg = { ...packageJson, ...publishConfig };
121+
122+
for (const [key, value] of Object.entries(publishConfig)) {
123+
if (value === null) {
124+
delete updatedPkg[key];
125+
}
126+
}
127+
128+
await writeFile(pkg, JSON.stringify(updatedPkg, null, 2), { encoding: 'utf8' });
129+
}
157130
}
158-
159-
await cd(EXPERIMENT_BENCH_DIR);
160-
await $`pnpm vite build`;
161-
});
131+
}
162132

163133
console.info({
164134
control: controlBranchName,
@@ -167,6 +137,18 @@ console.info({
167137
CONTROL_DIR,
168138
});
169139

140+
// setup experiment
141+
await within(async () => {
142+
await buildRepo(EXPERIMENT_DIR, experimentRef);
143+
});
144+
145+
if (!REUSE_CONTROL) {
146+
// setup control
147+
await within(async () => {
148+
await buildRepo(CONTROL_DIR, controlRef);
149+
});
150+
}
151+
170152
// start build assets
171153
$`cd ${CONTROL_BENCH_DIR} && pnpm vite preview --port ${CONTROL_PORT}`;
172154
$`cd ${EXPERIMENT_BENCH_DIR} && pnpm vite preview --port ${EXPERIMENT_PORT}`;
@@ -194,3 +176,40 @@ try {
194176
}
195177

196178
process.exit(0);
179+
180+
/**
181+
* @param {string} directory the directory to clone into
182+
* @param {string} ref the ref to checkout
183+
*/
184+
async function buildRepo(directory, ref) {
185+
// the benchmark directory is located in `packages/@glimmer/benchmark` in each of the
186+
// experiment and control checkouts
187+
const benchDir = join(directory, 'benchmark', 'benchmarks', 'krausest');
188+
189+
await cd(directory);
190+
191+
// write the `pwd` to the output to make it easier to debug if something goes wrong
192+
await $`pwd`;
193+
194+
// clone the raw git repo for the experiment
195+
await $`git clone ${join(ROOT, '.git')} .`;
196+
197+
// checkout the repo to the HEAD of the current branch
198+
await $`git checkout --force ${ref}`;
199+
200+
// recreate the benchmark directory
201+
await $`rm -rf ./benchmark`;
202+
// intentionally use the same folder for both experiment and control
203+
await $`cp -r ${BENCHMARK_FOLDER} ./benchmark`;
204+
205+
// `pnpm install` and build the repo
206+
await $`pnpm install --no-frozen-lockfile`;
207+
await $`pnpm build`;
208+
209+
// rewrite all `package.json`s to behave like published packages
210+
await rewritePackageJson();
211+
212+
// build the benchmarks using vite
213+
await cd(benchDir);
214+
await $`pnpm vite build`;
215+
}

‎bin/tsconfig.json

-3
Original file line numberDiff line numberDiff line change
@@ -2,18 +2,15 @@
22
"compilerOptions": {
33
"composite": true,
44
"skipLibCheck": true,
5-
65
"baseUrl": ".",
76
"allowJs": true,
87
"checkJs": true,
98
"outDir": "../ts-dist/bin",
10-
119
"target": "es2020",
1210
"module": "esnext",
1311
"moduleResolution": "bundler",
1412
"allowSyntheticDefaultImports": true,
1513
"verbatimModuleSyntax": true,
16-
1714
"strict": true,
1815
"suppressImplicitAnyIndexErrors": false,
1916
"useDefineForClassFields": false,

‎packages/@glimmer-workspace/benchmark-env/package.json

+1
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
"version": "0.84.3",
44
"private": true,
55
"repository": "https://github.com/glimmerjs/glimmer-vm/tree/main/packages/@glimmer-workspace/benchmark-env",
6+
"type": "module",
67
"main": "index.ts",
78
"scripts": {
89
"test:lint": "eslint .",

‎packages/@glimmer-workspace/integration-tests/test/chaos-rehydration-test.ts

+1-2
Original file line numberDiff line numberDiff line change
@@ -89,11 +89,10 @@ abstract class AbstractChaosMonkeyTest extends RenderTest {
8989

9090
let removedNodeDisplay: Nullable<string>;
9191
switch (nodeToRemove.nodeType) {
92-
// eslint-disable-next-line @typescript-eslint/no-unsafe-enum-comparison
9392
case COMMENT_NODE:
9493
removedNodeDisplay = `<!--${nodeToRemove.nodeValue}-->`;
9594
break;
96-
// eslint-disable-next-line @typescript-eslint/no-unsafe-enum-comparison
95+
9796
case ELEMENT_NODE:
9897
removedNodeDisplay = castToBrowser(nodeToRemove, ['HTML', 'SVG']).outerHTML;
9998
break;

‎packages/@glimmer/compiler/package.json

+17-17
Original file line numberDiff line numberDiff line change
@@ -4,29 +4,29 @@
44
"license": "MIT",
55
"repository": "https://github.com/glimmerjs/glimmer-vm/tree/main/packages/@glimmer/compiler",
66
"type": "module",
7-
"main": "index.ts",
8-
"types": "index.ts",
9-
"exports": {
10-
"types": "./index.ts",
11-
"development": "./index.ts",
12-
"import": "./dist/index.js"
13-
},
7+
"exports": "./index.ts",
148
"publishConfig": {
159
"access": "public",
16-
"types": "dist/dev/index.d.ts",
1710
"exports": {
1811
".": {
19-
"types": "./dist/dev/index.d.ts",
20-
"development": {
21-
"require": "./dist/dev/index.cjs",
22-
"import": "./dist/dev/index.js"
12+
"require": {
13+
"development": {
14+
"types": "./dist/dev/index.d.cts",
15+
"default": "./dist/dev/index.cjs"
16+
}
2317
},
24-
"require": "./dist/dev/index.cjs",
25-
"import": "./dist/dev/index.js"
18+
"default": {
19+
"development": {
20+
"types": "./dist/dev/index.d.ts",
21+
"default": "./dist/dev/index.js"
22+
},
23+
"default": {
24+
"types": "./dist/prod/index.d.ts",
25+
"default": "./dist/prod/index.js"
26+
}
27+
}
2628
}
27-
},
28-
"main": null,
29-
"module": "dist/dev/index.js"
29+
}
3030
},
3131
"files": [
3232
"dist"

‎packages/@glimmer/debug-util/package.json

+1-24
Original file line numberDiff line numberDiff line change
@@ -6,32 +6,9 @@
66
"description": "Common utilities used in Glimmer with debug-specific behavior",
77
"repository": "https://github.com/tildeio/glimmer/tree/main/packages/@glimmer/debug-util",
88
"type": "module",
9-
"main": "index.ts",
10-
"types": "index.ts",
11-
"publishConfig": {
12-
"access": "public",
13-
"types": "dist/dev/index.d.ts",
14-
"exports": {
15-
".": {
16-
"types": "./dist/dev/index.d.ts",
17-
"development": {
18-
"require": "./dist/dev/index.cjs",
19-
"import": "./dist/dev/index.js"
20-
},
21-
"require": "./dist/dev/index.cjs",
22-
"import": "./dist/prod/index.js"
23-
}
24-
},
25-
"main": null,
26-
"module": "dist/dev/index.js"
27-
},
28-
"files": [
29-
"dist"
30-
],
9+
"exports": "./index.ts",
3110
"scripts": {
32-
"build": "rollup -c rollup.config.mjs",
3311
"test:lint": "eslint .",
34-
"test:publint": "publint",
3512
"test:types": "tsc --noEmit -p ../tsconfig.json"
3613
},
3714
"dependencies": {

0 commit comments

Comments
 (0)