Skip to content

Commit

Permalink
Improve benchmark setup (#1647)
Browse files Browse the repository at this point in the history
* Improve benchmark setup

The main improvement is cloning from your local `.git` directory rather
than the remote.

There are also some minor ergonomic improvements to the output.

* Fix lint error

* Fix typescript errors in js files

* Silence a bunch of lint warnings
  • Loading branch information
wycats authored Oct 31, 2024
1 parent 153afe9 commit 7f4f3f0
Show file tree
Hide file tree
Showing 38 changed files with 169 additions and 124 deletions.
22 changes: 17 additions & 5 deletions benchmark/benchmarks/krausest/vite.config.mts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,10 @@ const packagePath = (name: string) => {
};

export default defineConfig({
plugins: [benchmark()],
plugins: [importMeta(), benchmark()],
preview: {
strictPort: true,
},
resolve: {
alias: {
'@glimmer-workspace/benchmark-env': '@glimmer-workspace/benchmark-env/index.ts',
Expand All @@ -27,22 +30,31 @@ export default defineConfig({
},
});

function importMeta(): Plugin {
return {
name: 'define custom import.meta.env',
async transform(code) {
if (code.includes('import.meta.env.VM_LOCAL_DEV')) {
return code.replace(/import.meta.env.VM_LOCAL_DEV/g, 'false');
}
return undefined;
},
enforce: 'pre',
};
}

function benchmark(): Plugin {
return {
enforce: 'pre',
name: '@glimmer/benchmark',
resolveId(id) {
if (id === '@glimmer/env') {
return '\0@glimmer/env';
} else if (id === '@glimmer/local-debug-flags') {
return '\0@glimmer/local-debug-flags';
}
},
load(id) {
if (id === '\0@glimmer/env') {
return `export const DEBUG = false;`;
} else if (id === '\0@glimmer/local-debug-flags') {
return `export const LOCAL_SHOULD_LOG = false;`;
}
/** @type {string | undefined} */
let result: string | undefined;
Expand Down
46 changes: 18 additions & 28 deletions bin/setup-bench.mjs
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
import 'zx/globals';
import os from 'node:os';
import { join } from 'node:path';
import chalk from 'chalk';

const ROOT = new URL('..', import.meta.url).pathname;
$.verbose = true;

/*
Expand Down Expand Up @@ -80,21 +84,6 @@ const isMacOs = os.platform() === 'darwin';

const BENCHMARK_FOLDER = join(pwd, benchmarkFolder);

const rawUpstreamUrl = await $`git ls-remote --get-url upstream`;
const rawOriginUrl = await $`git ls-remote --get-url origin`;
let originUrlStr = rawOriginUrl.toString().trim();
let upstreamUrlStr = rawUpstreamUrl.toString().trim();

if (upstreamUrlStr === 'upstream') {
// if we not inside fork, falling back to origin
upstreamUrlStr = originUrlStr;
}

if (FORK_NAME && FORK_NAME !== 'glimmerjs/glimmer-vm') {
// if PR from fork, we need to resolve fork's commit
originUrlStr = originUrlStr.replace('glimmerjs/glimmer-vm', FORK_NAME);
}

const CONTROL_PORT = 4020;
const EXPERIMENT_PORT = 4021;
const CONTROL_URL = `http://localhost:${CONTROL_PORT}`;
Expand All @@ -105,15 +94,15 @@ const EXPERIMENT_URL = `http://localhost:${EXPERIMENT_PORT}`;
// setup experiment
await within(async () => {
await cd(EXPERIMENT_DIR);
await $`git clone ${originUrlStr} .`;
await $`git clone ${join(ROOT, '.git')} .`;
await $`git checkout ${experimentBranchName}`;
await $`rm -rf ./benchmark`;
await $`cp -r ${BENCHMARK_FOLDER} ./benchmark`;

console.info('installing experiment source');
await $`pnpm install --no-frozen-lockfile`.quiet();
console.info('building experiment source, may take a while');
await $`pnpm build`.quiet();
console.info(`$ pnpm install --frozen-lockfile ${chalk.gray('[experiment]')}`);
await $`pnpm install --frozen-lockfile`.quiet();
console.info(`$ pnpm build ${chalk.gray('[experiment]')}`);
await spinner(() => $`pnpm build`.quiet());

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' {} \\;`;
Expand All @@ -132,15 +121,16 @@ await within(async () => {
// setup control
await within(async () => {
await cd(CONTROL_DIR);
await $`git clone ${upstreamUrlStr} .`;
await $`git checkout ${controlBranchName}`;
await $`git clone ${join(ROOT, '.git')} .`;
await $`git fetch origin`;
await $`git reset --hard origin/${controlBranchName}`;
await $`rm -rf ./benchmark`;
await $`cp -r ${BENCHMARK_FOLDER} ./benchmark`;

console.info('installing control source');
await $`pnpm install --no-frozen-lockfile`.quiet();
console.info('building control source, may take a while');
await $`pnpm build`.quiet();
console.info(`$ pnpm install --frozen-lockfile ${chalk.gray('[control]')}`);
await $`pnpm install --frozen-lockfile`.quiet();
console.info(`$ pnpm build ${chalk.gray('[control]')}`);
await spinner(() => $`pnpm build`.quiet());

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' {} \\;`;
Expand All @@ -157,8 +147,8 @@ await within(async () => {
});

console.info({
upstreamUrlStr,
originUrlStr,
control: controlBranchName,
experiment: experimentBranchName,
EXPERIMENT_DIR,
CONTROL_DIR,
});
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@
"typescript": "^5.0.4",
"vite": "^5.0.12",
"xo": "^0.54.2",
"zx": "^7.2.3"
"zx": "^8.1.9"
},
"changelog": {
"repo": "glimmerjs/glimmer-vm",
Expand Down
7 changes: 3 additions & 4 deletions packages/@glimmer-workspace/build/lib/config.js
Original file line number Diff line number Diff line change
Expand Up @@ -256,9 +256,8 @@ export class Package {
}

/**
* @typedef {object} Formats
* @property {boolean} [ esm ] enabled by default
* @property {boolean} [ cjs ] enabled by default until eslint-plugin-ember and ember-source no longer need it
* @typedef {{esm?: boolean, cjs?: boolean}} Formats
* @param {Formats} [formats] enabled by default
*
* @returns {import("rollup").RollupOptions[] | import("rollup").RollupOptions}
*/
Expand Down Expand Up @@ -371,7 +370,7 @@ export class Package {
typescript(this.#package, {
target: ScriptTarget.ES2021,
module: ModuleKind.CommonJS,
moduleResolution: ModuleResolutionKind.NodeJs,
moduleResolution: ModuleResolutionKind.Node16,
}),
],
}));
Expand Down
1 change: 1 addition & 0 deletions packages/@glimmer-workspace/test-utils/lib/guard.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import type { Expand, Maybe, Present } from '@glimmer/interfaces';
import { isPresent } from '@glimmer/util';

// eslint-disable-next-line @typescript-eslint/no-explicit-any
export type NTuple<N extends number, Type, T extends any[] = []> = T['length'] extends N
? T
: NTuple<N, Type, [...T, Type]>;
Expand Down
4 changes: 1 addition & 3 deletions packages/@glimmer-workspace/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -2,16 +2,14 @@
"compilerOptions": {
"composite": true,
"baseUrl": "..",

"allowJs": true,
"target": "esnext",
"module": "esnext",
"moduleResolution": "bundler",
"allowSyntheticDefaultImports": true,
"experimentalDecorators": true,
"verbatimModuleSyntax": true,

"outDir": "../../ts-dist/@glimmer-workspace",

"strict": true,
"suppressImplicitAnyIndexErrors": false,
"useDefineForClassFields": false,
Expand Down
2 changes: 1 addition & 1 deletion packages/@glimmer/debug/lib/debug.ts
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ export function debug(
let metadata = opcodeMetadata(op.type, isMachine);

if (!metadata) {
throw new Error(`Missing Opcode Metadata for ${op}`);
throw new Error(`Missing Opcode Metadata for ${op.type}`);
}

let out = Object.create(null);
Expand Down
1 change: 1 addition & 0 deletions packages/@glimmer/debug/lib/metadata.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ export const OPERAND_TYPES = [
];

function isOperandType(s: string): s is OperandType {
// eslint-disable-next-line @typescript-eslint/no-explicit-any
return OPERAND_TYPES.indexOf(s as any) !== -1;
}

Expand Down
1 change: 1 addition & 0 deletions packages/@glimmer/debug/lib/stack-check.ts
Original file line number Diff line number Diff line change
Expand Up @@ -290,6 +290,7 @@ class SafeStringChecker implements Checker<SafeString> {

validate(value: unknown): value is SafeString {
return (
// eslint-disable-next-line @typescript-eslint/no-explicit-any
typeof value === 'object' && value !== null && typeof (value as any).toHTML === 'function'
);
}
Expand Down
1 change: 1 addition & 0 deletions packages/@glimmer/global-context/index.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
/* eslint-disable @typescript-eslint/no-explicit-any */
/**
* This package contains global context functions for Glimmer. These functions
* are set by the embedding environment and must be set before initial render.
Expand Down
4 changes: 2 additions & 2 deletions packages/@glimmer/interfaces/lib/compile/wire-format/api.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ export type ExpressionSexpOpcodeMap = {
[TSexpOpcode in TupleExpression[0]]: Extract<TupleExpression, { 0: TSexpOpcode }>;
};

export interface SexpOpcodeMap extends ExpressionSexpOpcodeMap, StatementSexpOpcodeMap { }
export interface SexpOpcodeMap extends ExpressionSexpOpcodeMap, StatementSexpOpcodeMap {}
export type SexpOpcode = keyof SexpOpcodeMap;

export namespace Core {
Expand Down Expand Up @@ -372,7 +372,7 @@ export type SerializedTemplateBlock = [
hasDebug: boolean,
// upvars
upvars: string[],
lexicalSymbols?: string[]
lexicalSymbols?: string[],
];

/**
Expand Down
1 change: 1 addition & 0 deletions packages/@glimmer/interfaces/lib/runtime/arguments.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ export interface ArgumentsDebug {
}

export interface ArgumentError {
// eslint-disable-next-line @typescript-eslint/no-explicit-any
error: any;
}

Expand Down
1 change: 1 addition & 0 deletions packages/@glimmer/interfaces/lib/runtime/environment.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,5 +45,6 @@ export interface Environment {

isInteractive: boolean;
debugRenderTree?: DebugRenderTree | undefined;
// eslint-disable-next-line @typescript-eslint/no-explicit-any
isArgumentCaptureError?: ((error: any) => boolean) | undefined;
}
1 change: 1 addition & 0 deletions packages/@glimmer/manager/lib/internal/defaults.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ type FnArgs<Args extends Arguments = Arguments> =
| [...Args['positional'], Args['named']]
| [...Args['positional']];

// eslint-disable-next-line @typescript-eslint/no-explicit-any
type AnyFunction = (...args: any[]) => unknown;

interface State {
Expand Down
2 changes: 2 additions & 0 deletions packages/@glimmer/manager/lib/public/component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,7 @@ export class CustomComponentManager<O extends Owner, ComponentInstance>
throw new Error(
`Custom component managers must have a \`capabilities\` property that is the result of calling the \`capabilities('3.13')\` (imported via \`import { capabilities } from '@ember/component';\`). Received: \`${JSON.stringify(
delegate.capabilities
// eslint-disable-next-line @typescript-eslint/no-base-to-string
)}\` for: \`${delegate}\``
);
}
Expand All @@ -150,6 +151,7 @@ export class CustomComponentManager<O extends Owner, ComponentInstance>
}

getDebugName(definition: ComponentDefinitionState): string {
// eslint-disable-next-line @typescript-eslint/no-base-to-string
return typeof definition === 'function' ? definition.name : definition.toString();
}

Expand Down
1 change: 1 addition & 0 deletions packages/@glimmer/manager/lib/public/helper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@ export class CustomHelperManager<O extends Owner = Owner> implements InternalHel
throw new Error(
`Custom helper managers must have a \`capabilities\` property that is the result of calling the \`capabilities('3.23')\` (imported via \`import { capabilities } from '@ember/helper';\`). Received: \`${JSON.stringify(
delegate.capabilities
// eslint-disable-next-line @typescript-eslint/no-base-to-string
)}\` for: \`${delegate}\``
);
}
Expand Down
1 change: 1 addition & 0 deletions packages/@glimmer/manager/lib/public/modifier.ts
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ export class CustomModifierManager<O extends Owner, ModifierInstance>
throw new Error(
`Custom modifier managers must have a \`capabilities\` property that is the result of calling the \`capabilities('3.22')\` (imported via \`import { capabilities } from '@ember/modifier';\`). Received: \`${JSON.stringify(
delegate.capabilities
// eslint-disable-next-line @typescript-eslint/no-base-to-string
)}\` for: \`${delegate}\``
);
}
Expand Down
1 change: 1 addition & 0 deletions packages/@glimmer/manager/lib/util/args-proxy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,7 @@ class PositionalArgsProxy implements ProxyHandler<[]> {
return valueForRef(positional[parsed]!);
}

// eslint-disable-next-line @typescript-eslint/no-explicit-any
return (target as any)[prop];
}

Expand Down
Loading

0 comments on commit 7f4f3f0

Please sign in to comment.