fix: cleanup options when suite finishes

package.json

@@ -40,6 +40,7 @@
     "@eslint/js": "^9.15.0",
     "@types/node": "^22.10.2",
     "@types/yargs": "^17.0.33",
+    "@types/debug": "^4.1.12",
     "dotenv": "^10.0.0",
     "eslint": "^9.15.0",
     "eslint-config-prettier": "^9.1.0",
@@ -62,6 +63,7 @@
     "aws-sdk": "^2.932.0",
     "csv-parse": "^5.6.0",
     "csv-stringify": "^6.5.2",
+    "debug": "^4.4.0",
     "glob": "^10.4.5",
     "log-symbols": "^7.0.0",
     "yaml": "^2.7.0",

src/benchmark/benchmarkFn.ts

@@ -65,8 +65,21 @@ export const bench: BenchApi = createBenchmarkFunction(function <T, T2>(
     },
   });
 
+  const {id: _, ...optionsWithoutId} = opts;
   setFn(task, handler);
-  store.setOptions(task, opts);
+  store.setOptions(task, optionsWithoutId);
+
+  task.onFinished = [
+    () => {
+      store.removeOptions(task);
+    },
+    () => {
+      // Clear up the assigned handler to clean the memory
+      // eslint-disable-next-line @typescript-eslint/ban-ts-comment
+      // @ts-expect-error
+      setFn(task, null);
+    },
+  ];
 });
 
 function createBenchmarkFunction(
@@ -123,7 +136,12 @@ function coerceToOptsObj<T, T2>(
  * ```
  */
 export function setBenchOpts(opts: BenchmarkOpts): void {
-  store.setOptions(getCurrentSuite(), opts);
+  const suite = getCurrentSuite();
+  store.setOptions(suite, opts);
+
+  suite.on("afterAll", () => {
+    store.removeOptions(suite);
+  });
 }
 
 export const setBenchmarkOptions = setBenchOpts;

src/benchmark/globalState.ts

@@ -1,6 +1,9 @@
+import Debug from "debug";
 import type {Suite, SuiteCollector, Task} from "@vitest/runner";
 import {BenchmarkResult, BenchmarkOpts, BenchmarkResults} from "../types.js";
 
+const debug = Debug("@chainsafe/benchmark/state");
+
 /**t
  * Map of results by root suite.
  */
@@ -14,13 +17,14 @@ let globalOpts: BenchmarkOpts | undefined;
 /**
  * Map to persist options set in describe blocks
  */
-const optsMap = new WeakMap<object, BenchmarkOpts>();
+const optsMap = new Map<object, BenchmarkOpts>();
 
 export const store = {
   getResult(id: string): BenchmarkResult | undefined {
     return results.get(id);
   },
   setResult(id: string, result: BenchmarkResult): void {
+    debug("setting result for %o", id);
     results.set(id, result);
   },
   getAllResults(): BenchmarkResults {
@@ -30,12 +34,17 @@ export const store = {
     return optsMap.get(suite);
   },
   setOptions(suite: Task | Suite | SuiteCollector, opts: BenchmarkOpts): void {
+    if (Object.keys(opts).length === 0) return;
+
+    debug("setting options for %o with name %o %O", suite.type, suite.name, opts);
     optsMap.set(suite, opts);
   },
-  removeOptions(suite: Task | Suite): void {
+  removeOptions(suite: Task | Suite | SuiteCollector): void {
+    debug("removing options for %o with name %o", suite.type, suite.name);
     optsMap.delete(suite);
   },
   setGlobalOptions(opts: Partial<BenchmarkOpts>): void {
+    debug("setting global options %O", opts);
     globalOpts = opts;
   },
   getGlobalOptions(): BenchmarkOpts | undefined {

src/benchmark/runner.ts

@@ -6,13 +6,15 @@ import {
   VitestRunner,
   VitestRunnerConfig,
   VitestRunnerImportSource,
-  setFn,
 } from "@vitest/runner";
 import path from "node:path";
+import Debug from "debug";
 import {Benchmark, BenchmarkOpts, BenchmarkResults} from "../types.js";
 import {BenchmarkReporter} from "./reporter.js";
 import {store} from "./globalState.js";
 
+const debug = Debug("@chainsafe/benchmark/runner");
+
 export class BenchmarkRunner implements VitestRunner {
   readonly triggerGC: boolean;
   readonly config: VitestRunnerConfig;
@@ -43,24 +45,26 @@ export class BenchmarkRunner implements VitestRunner {
 
   onAfterRunSuite(suite: Suite): void {
     this.reporter.onSuiteFinished(suite);
-    store.removeOptions(suite);
   }
 
   onBeforeRunTask(task: Task): void {
     this.reporter.onTestStarted(task);
   }
 
-  async onAfterRunTask(task: Task): Promise<void> {
+  async onTaskFinished(task: Task): Promise<void> {
     this.reporter.onTestFinished(task);
-    store.removeOptions(task);
 
-    if (task.type === "test" || task.type === "custom") {
-      // Clear up the assigned handler to clean the memory
-      // eslint-disable-next-line @typescript-eslint/ban-ts-comment
-      // @ts-expect-error
-      setFn(task, null);
+    // To help maintain consistent memory usage patterns
+    // we trigger garbage collection manually
+    if (this.triggerGC && global.gc) {
+      global.gc();
+      // Make sure the syn operation is off the event loop
+      await new Promise((resolve) => setTimeout(resolve, 0));
     }
+  }
 
+  // eslint-disable-next-line @typescript-eslint/no-unused-vars
+  async onAfterRunTask(task: Task): Promise<void> {
     // To help maintain consistent memory usage patterns
     // we trigger garbage collection manually
     if (this.triggerGC && global.gc) {
@@ -86,12 +90,15 @@ export class BenchmarkRunner implements VitestRunner {
   async process(files: string[]): Promise<BenchmarkResults> {
     store.setGlobalOptions(this.benchmarkOpts);
 
+    debug("starting tests %O", files);
     const res = await startTests(files, this);
 
     const passed = res.filter((r) => r.result?.state == "pass");
     const skipped = res.filter((r) => r.result?.state == "skip");
     const failed = res.filter((r) => r.result?.state == "fail");
 
+    debug("finished tests. passed: %i, skipped: %i, failed: %i", passed.length, skipped.length, failed.length);
+
     if (failed.length > 0) {
       throw failed[0].result?.errors;
     }

src/cli/cli.ts

@@ -2,9 +2,12 @@
 import yargs from "yargs";
 import fs from "node:fs";
 import path from "node:path";
+import Debug from "debug";
 import {parse as parseYaml} from "yaml";
 import {hideBin} from "yargs/helpers";
 
+const debug = Debug("@chainsafe/benchmark/cli");
+
 import {benchmarkOptions, CLIOptions, fileCollectionOptions, storageOptions} from "./options.js";
 import {run} from "./run.js";
 import {compare} from "./compare.js";
@@ -20,6 +23,7 @@ void yargs(hideBin(process.argv))
     },
     handler: async (argv) => {
       const cliOpts = {...argv} as unknown as CLIOptions & {spec: string[]};
+      debug("Executing command run with %O", cliOpts);
 
       await run(cliOpts);
     },
@@ -33,6 +37,7 @@ void yargs(hideBin(process.argv))
     },
     handler: async (argv) => {
       const cliOpts = {...argv} as unknown as {dirs: string[]};
+      debug("Executing command compare with %O", cliOpts);
 
       await compare(cliOpts);
     },

src/cli/run.ts

@@ -1,4 +1,5 @@
 import * as github from "@actions/github";
+import Debug from "debug";
 import {getHistoryProvider} from "../history/index.js";
 import {resolveShouldPersist} from "../history/shouldPersist.js";
 import {validateBenchmark} from "../history/schema.js";
@@ -22,6 +23,8 @@ import {HistoryProviderType} from "../history/provider.js";
 import {performanceReportComment} from "../github/comments/performanceReportComment.js";
 import {GithubCommentTag} from "../github/octokit.js";
 
+const debug = Debug("@chainsafe/benchmark/cli");
+
 export async function run(opts_: FileCollectionOptions & StorageOptions & BenchmarkOpts): Promise<void> {
   const opts = Object.assign({}, optionsDefault, opts_);
 
@@ -70,6 +73,8 @@ export async function run(opts_: FileCollectionOptions & StorageOptions & Benchm
     // Persist new benchmark data
     const currentBranch = await getCurrentBranch();
     const shouldPersist = await resolveShouldPersist(opts, currentBranch);
+
+    debug("detecting to persist results. found: %o", shouldPersist);
     if (shouldPersist === true) {
       const branch =
         currentCommit.branch ??
@@ -86,6 +91,7 @@ export async function run(opts_: FileCollectionOptions & StorageOptions & Benchm
 
     const resultsComp = computePerformanceReport(currBench, prevBench, opts.threshold);
 
+    debug("detecting to post comment. skipPostComment: %o, isGaRun: %o", !opts.skipPostComment, isGaRun());
     if (!opts.skipPostComment && isGaRun()) {
       await postGaComment({
         commentBody: performanceReportComment(resultsComp),

test/perf/iteration.test.ts

@@ -35,8 +35,8 @@ describe("Array iteration", () => {
   bench({
     id: "sum array with reduce beforeEach",
     beforeEach: () => Array.from({length: 1e4}, (_, i) => i),
-    fn: () => {
-      arr.reduce((total, curr) => total + curr, 0);
+    fn: (arrayFromBeforeEach) => {
+      arrayFromBeforeEach.reduce((total, curr) => total + curr, 0);
 
       // Uncomment below to cause a guaranteed performance regression
       // arr.reduce((total, curr) => total + curr, 0);
@@ -47,26 +47,17 @@ describe("Array iteration", () => {
   bench({
     id: "sum array with reduce before beforeEach",
     before: () => Array.from({length: 1e4}, (_, i) => i),
-    beforeEach: (arr) => arr.slice(0),
-    fn: () => {
-      arr.reduce((total, curr) => total + curr, 0);
+    beforeEach: (arrFromBefore) => arrFromBefore.slice(0),
+    fn: (arrayFromBeforeEach) => {
+      arrayFromBeforeEach.reduce((total, curr) => total + curr, 0);
 
       // Uncomment below to cause a guaranteed performance regression
       // arr.reduce((total, curr) => total + curr, 0);
       // arr.reduce((total, curr) => total + curr, 0);
     },
   });
 
-  bench.skip("sum array with reduce", () => {
-    arr.reduce((total, curr) => total + curr, 0);
-  });
-
-  // bench.only("sum array with reduce", () => {
-  //   arr.reduce((total, curr) => total + curr, 0);
-  // });
-
   // Reporter options
-
   bench({
     id: "sum array with reduce high threshold",
     threshold: 5,

yarn.lock

@@ -768,6 +768,13 @@
   resolved "https://registry.yarnpkg.com/@tsconfig/node16/-/node16-1.0.4.tgz#0b92dcc0cc1c81f6f306a381f28e31b1a56536e9"
   integrity sha512-vxhUy4J8lyeyinH7Azl1pdd43GJhZH/tP2weN8TntQblOY+A0XbT8DJk1/oCPuOOyg/Ja757rG0CgHcWC8OfMA==
 
+"@types/debug@^4.1.12":
+  version "4.1.12"
+  resolved "https://registry.yarnpkg.com/@types/debug/-/debug-4.1.12.tgz#a155f21690871953410df4b6b6f53187f0500917"
+  integrity sha512-vIChWdVG3LG1SMxEvI/AK+FWJthlrqlTu7fbrlywTkkaONwk/UAGaULXRlf8vkzFBLVm0zkMdCquhL5aOjhXPQ==
+  dependencies:
+    "@types/ms" "*"
+
 "@types/estree@1.0.6", "@types/estree@^1.0.0", "@types/estree@^1.0.6":
   version "1.0.6"
   resolved "https://registry.yarnpkg.com/@types/estree/-/estree-1.0.6.tgz#628effeeae2064a1b4e79f78e81d87b7e5fc7b50"
@@ -783,6 +790,11 @@
   resolved "https://registry.yarnpkg.com/@types/json5/-/json5-0.0.29.tgz#ee28707ae94e11d2b827bcbe5270bcea7f3e71ee"
   integrity sha1-7ihweulOEdK4J7y+UnC86n8+ce4=
 
+"@types/ms@*":
+  version "2.1.0"
+  resolved "https://registry.yarnpkg.com/@types/ms/-/ms-2.1.0.tgz#052aa67a48eccc4309d7f0191b7e41434b90bb78"
+  integrity sha512-GsCCIZDE/p3i96vtEqx+7dBUGXrc7zeSK3wwPHIaRThS+9OhWIXRqzs4d6k1SVU8g91DrNRWxWUGhp5KXQb2VA==
+
 "@types/node@^22.10.2":
   version "22.10.2"
   resolved "https://registry.yarnpkg.com/@types/node/-/node-22.10.2.tgz#a485426e6d1fdafc7b0d4c7b24e2c78182ddabb9"
@@ -1331,7 +1343,7 @@ data-view-byte-offset@^1.0.0:
     es-errors "^1.3.0"
     is-data-view "^1.0.1"
 
-debug@4, debug@^4.3.7:
+debug@4, debug@^4.3.7, debug@^4.4.0:
   version "4.4.0"
   resolved "https://registry.yarnpkg.com/debug/-/debug-4.4.0.tgz#2b3f2aea2ffeb776477460267377dc8710faba8a"
   integrity sha512-6WTZ/IxCY/T6BALoZHaE4ctp9xm+Z5kY/pzYaCHRFeyVhojxlrm+46y68HA6hr0TcwEssoxNiDEUJQjfPZ/RYA==