Benchmarks in CI.

[shimkiv]

Closes #1473
Closes #1487
Closes #1489
Closes o1-labs/benchmark-infra#1

[mitschabaude]

Hey @shimkiv one ask - can we separate the core benchmarking library from the tooling to upload benchmark data?
I.e. have two library files

• benchmark.ts similar to original
• benchmarking-infra.ts (or whatever you want to call it), which depends on influx db client etc

Similarly, it would be nice if the benchmark definitions like EcdsaBenchmark would be in a different file from the code that runs them and stores results.

Goal: I want to keep the ability to easily run these benchmarks locally and print out their results, by writing a small script that calls Benchmark.run(), which does not depend on and interacts with any of our infra

[shimkiv]

Hey @shimkiv one ask - can we separate the core benchmarking library from the tooling to upload benchmark data? I.e. have two library files
Goal: I want to keep the ability to easily run these benchmarks locally and print out their results, by writing a small script that calls Benchmark.run(), which does not depend on and interacts with any of our infra

Hey @mitschabaude, if you will run benchmarks locally as it was intended at first like ./run benchmarks/ecdsa.ts --bundle then it won't talk to any of infra since it won't be configured properly.
All the difference really in results logging, everything else is the same.

[mitschabaude]

it won't talk to any of infra since it won't be configured properly.

Ok thanks I see it now in the code that nothing happens if you don't configure influx db.

Still, I'd like to keep the infra code separate from the core benchmark code. This is cleaner from a code organization perspective. The core benchmark lib is not supposed to:

• require '@influxdata/influxdb-client' as an npm dependency
• try to read process.env
• require running in Node.js

It should be a pure JS lib really with no external dependencies (right now it depends on jsstat but that is a pure math library itself)

[mitschabaude]

@shimkiv you could pass in the additional environment data like the CPU to Benchmark.run(), so that the benchmarking lib doesn't have to collect that data internally:

import os from "node:os";
// import benchmarks

let environment = {
  hardware: `${os.cpus()[0].model}, ${os.cpus().length} cores, ...`
  // ...
 };
 
let results = [
  ...await EcdsaBenchmark.run(environment),
  ...await SomeOtherBenchmark.run(environment),
];
 
// results include measurements + environment data

[shimkiv]

@shimkiv you could pass in the additional environment data like the CPU to Benchmark.run(), so that the benchmarking lib doesn't have to collect that data internally:

This is cool feature but since we run all of them on the same env. it looks like we transfer the same data back and forth without any meaning. In its current state. But might be helpful after refactoring you suggest. Thanks.

[mitschabaude]

@shimkiv you could pass in the additional environment data like the CPU to Benchmark.run(), so that the benchmarking lib doesn't have to collect that data internally:

This is cool feature but since we run all of them on the same env. it looks like we transfer the same data back and forth without any meaning. In its current state. But might be helpful after refactoring you suggest. Thanks.

I agree, it's just passing data through. The alternative is to add the environment data to the benchmark results after collecting them.

In any case, the data needs to come from outside somehow if we go with the requirement that the benchmarking lib doesn't collect that data itself

[shimkiv]

@mitschabaude can you please take another look on latest refactoring in attempt to reflect what was requested (from my understanding).
We can also implement the file-system historical data preservation for local runs, but probably it should be done later in another PR.

[ymekuria]

Nice work @shimkiv! I will leave it up to @mitschabaude to decide if theses changes addressed his comments.

[mitschabaude]

some comments left, looks great overall!

[mitschabaude]

ship it 🚢