Skip to content

Commit

Permalink
Addressing review comments.
Browse files Browse the repository at this point in the history
  • Loading branch information
shimkiv committed Apr 20, 2024
1 parent 8deb216 commit bfe34a3
Show file tree
Hide file tree
Showing 9 changed files with 75 additions and 74 deletions.
3 changes: 2 additions & 1 deletion .github/workflows/benchmarks.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
4 changes: 2 additions & 2 deletions benchmark/benchmarks/ecdsa.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down
22 changes: 0 additions & 22 deletions benchmark/benchmarks/init.ts

This file was deleted.

32 changes: 32 additions & 0 deletions benchmark/runners/init.ts
Original file line number Diff line number Diff line change
@@ -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);
21 changes: 7 additions & 14 deletions benchmark/runners/simple.ts
Original file line number Diff line number Diff line change
@@ -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
* ```
Expand All @@ -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);
33 changes: 0 additions & 33 deletions benchmark/runners/with-cloud-history.ts

This file was deleted.

11 changes: 10 additions & 1 deletion benchmark/utils/influxdb-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.');
}
}

Expand All @@ -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;
}
Expand Down Expand Up @@ -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);
}
}
20 changes: 20 additions & 0 deletions benchmark/utils/result-utils.ts
Original file line number Diff line number Diff line change
@@ -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');
}
}
3 changes: 2 additions & 1 deletion run-ci-benchmarks.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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

0 comments on commit bfe34a3

Please sign in to comment.