Skip to content
This repository has been archived by the owner on Sep 30, 2023. It is now read-only.

node and browser benchmark runner #32

Merged
merged 98 commits into from
May 31, 2021
Merged

Conversation

tabcat
Copy link
Contributor

@tabcat tabcat commented Apr 26, 2021

largish pr, apologies.

the changes are primarily

  • the addition of browser benchmarking
  • memory and cpu metrics
  • reports are printed to console and can be written in json or html formats
  • json reports can be compared to running benchmarks

much of the original source has been overwritten and restructured:

For each benchmark in dir/file path given, cli.js runs exec-benchmark.js inside of a nodejs Worker.
exec-benchmark.js imports exec-benchmark.node.js or exec-benchmark.browser.js to run the benchmark.

exec-benchmark.node.js is for running the benchmark in node, it only exports run.js which is what actually runs the benchmarks.

exec-benchmark.browser.js first bundles the benchmarks and run.js to be ran in the browser and hosts it on a webpack dev server (see webpack-server.js and webpack-entry.js). once the bundle is ready it visits the page with puppeteer and waits for the benchmarks to finish.

In both environments the benchmarks are recorded with benchmarker/server.js & benchmarker/client.js.
once benchmarks have been ran the results are given to reporter/index.js to generate a report. First the results are processed and then a report is printed to console. Optionally with a cli flag the reporter can write a json or html report to a given path.

JSON reports with another cli flag can be used to compare a new benchmark.
HTML reports show the metrics and some graphs of the metrics.

benchmark file structure has been changed. a benchmark file must export an object with a method named benchmark and benchmark file names should end in .benchmark.js. here are some examples:

Copy link
Contributor

@aphelionz aphelionz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great overall - some small changes that we discussed elsewhere. React review coming, and would be great to get @mistakia's thumbs up as well

Still need test and docs too...

Copy link

@geolffreym geolffreym left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@mistakia
Copy link
Contributor

The approach and code LGTM. Doing some testing using the ipfs-log and orbit-db benchmarks, will test it out by evaluating orbitdb-archive/ipfs-log#301

Copy link
Contributor

@aphelionz aphelionz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One more question here and I think we'll be good to go

@@ -4,16 +4,43 @@
"description": "OrbitDB Benchmark Runner",
"main": "./src/index.js",
"bin": {
"benchmark-runner": "./src/cli.js"
"benchmarker": "./src/cli.js"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we change the name of the repo to orbit-db-benchmarker? Thinking about consistency.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this could be changed to anything or kept the same, i chose the different name originally so it didnt conflict. probably good to have the package and repo the same name though (they are different right now; repo: benchmark-runner, package: orbit-db-benchmark-runner). shorter name probably better. also keep in mind the benchmarker doesnt have anything written in that makes it specific to benchmarking orbitdb, it can run benchmarks for anything.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alright let's just leave it as is for now until it becomes a real problem

@aphelionz aphelionz merged commit 76d38ba into orbitdb-archive:main May 31, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants