Addressing review comments.

Author: shimkiv

.github/workflows/benchmarks.yml

@@ -38,6 +38,7 @@ jobs:
           npm ci
           npm run build
           echo 'Running o1js benchmarks.'
-          node --enable-source-maps --stack-trace-limit=1000 src/build/run.js benchmark/runners/with-cloud-history.ts --bundle >>benchmarks.log 2>&1
+          node --enable-source-maps --stack-trace-limit=1000 src/build/run.js benchmark/runners/init.ts --bundle >>benchmarks.log 2>&1
+          node --enable-source-maps --stack-trace-limit=1000 src/build/run.js benchmark/runners/simple.ts --bundle >>benchmarks.log 2>&1
           cat benchmarks.log >> $GITHUB_STEP_SUMMARY
         shell: bash

benchmark/benchmarks/ecdsa.ts

@@ -11,14 +11,14 @@ import {
 } from '../../src/examples/crypto/ecdsa/ecdsa.js';
 import { benchmark } from '../benchmark.js';
 
-export { EcdsaBenchmark };
+export { EcdsaBenchmarks };
 
 let privateKey = Secp256k1.Scalar.random();
 let publicKey = Secp256k1.generator.scale(privateKey);
 let message = Bytes32.fromString("what's up");
 let signature = Ecdsa.sign(message.toBytes(), privateKey.toBigInt());
 
-const EcdsaBenchmark = benchmark(
+const EcdsaBenchmarks = benchmark(
   'ecdsa',
   async (tic, toc) => {
     tic('build constraint system');

benchmark/benchmarks/init.ts

@@ -0,0 +1,32 @@
+/**
+ * Specific runner for o1js initialization benchmarks to get "clean" metrics
+ *
+ * Run with
+ * ```
+ * ./run benchmark/runners/init.ts --bundle
+ * ```
+ */
+
+import { benchmark } from '../benchmark.js';
+import { processAndLogBenchmarkResults } from '../utils/result-utils.js';
+
+const InitBenchmarks = benchmark(
+  'init',
+  async (tic, toc) => {
+    tic('o1js import');
+    const { initializeBindings } = await import('o1js');
+    toc();
+
+    tic('bindings initialization');
+    await initializeBindings();
+    toc();
+  },
+  // Run once with no warmups to get the worst-case scenario metrics
+  { numberOfWarmups: 0, numberOfRuns: 1 }
+);
+
+// Run the initialization benchmarks and collect results
+const results = [...(await InitBenchmarks.run())];
+
+// Process and log results
+await processAndLogBenchmarkResults(results);

benchmark/runners/init.ts

@@ -1,6 +1,5 @@
 /**
- * Simple benchmarks runner
- * Exercises benchmarks and logs the results
+ * Simple benchmarks runner with historical data preservation in cloud storage (if configured)
  *
  * Run with
  * ```
@@ -9,20 +8,14 @@
  */
 
 import { initializeBindings } from 'o1js';
-import { logResult } from '../benchmark.js';
-import { EcdsaBenchmark } from '../benchmarks/ecdsa.js';
-import { InitBenchmark } from '../benchmarks/init.js';
+import { EcdsaBenchmarks } from '../benchmarks/ecdsa.js';
+import { processAndLogBenchmarkResults } from '../utils/result-utils.js';
 
 const results = [];
-
-// Run the initialization benchmark
-results.push(...(await InitBenchmark.run()));
-// Run all other benchmarks
 await initializeBindings();
-results.push(...(await EcdsaBenchmark.run()));
+
+// Run benchmarks and collect results
+results.push(...(await EcdsaBenchmarks.run()));
 
 // Process and log results
-for (const result of results) {
-  logResult(result);
-  console.log('\n');
-}
+await processAndLogBenchmarkResults(results);

benchmark/runners/simple.ts

@@ -56,7 +56,7 @@ export function writeResultToInfluxDb(result: BenchmarkResult): void {
       influxDbWriteClient.close();
     }
   } else {
-    console.info('Skipping writing to InfluxDB: client is not configured.');
+    debugLog('Skipping writing to InfluxDB: client is not configured.');
   }
 }
 
@@ -66,6 +66,7 @@ export function readPreviousResultFromInfluxDb(
   return new Promise((resolve) => {
     const { org, bucket } = INFLUXDB_CLIENT_OPTIONS;
     if (!influxDbClient || !org || !bucket) {
+      debugLog('Skipping querying InfluxDB: client is not configured.');
       resolve(undefined);
       return;
     }
@@ -130,3 +131,11 @@ export function readPreviousResultFromInfluxDb(
     }
   });
 }
+
+function debugLog(message: string): void {
+  // We can also use https://www.npmjs.com/package/debug
+  // should we need more configuration options over the debug logging in the future
+  if (process.env.DEBUG) {
+    console.log(message);
+  }
+}

benchmark/runners/with-cloud-history.ts

@@ -0,0 +1,20 @@
+/**
+ * Benchmark results processing utils
+ */
+
+import { BenchmarkResult, logResult } from '../benchmark.js';
+import {
+  readPreviousResultFromInfluxDb,
+  writeResultToInfluxDb,
+} from './influxdb-utils.js';
+
+export async function processAndLogBenchmarkResults(
+  results: BenchmarkResult[]
+): Promise<void> {
+  for (const result of results) {
+    const previousResult = await readPreviousResultFromInfluxDb(result);
+    logResult(result, previousResult);
+    writeResultToInfluxDb(result);
+    console.log('\n');
+  }
+}

benchmark/utils/influxdb-utils.ts

@@ -5,4 +5,5 @@ echo ""
 echo "Running o1js benchmarks."
 echo ""
 
-./run benchmark/runners/with-cloud-history.ts --bundle >>benchmarks.log 2>&1
+./run benchmark/runners/init.ts --bundle >>benchmarks.log 2>&1
+./run benchmark/runners/simple.ts --bundle >>benchmarks.log 2>&1