Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rework loading benchmark to avoid v8 caching #36

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

alexeyraspopov
Copy link
Owner

Benchmarks are quite random by themselves but there are things we must do to make them close to real numbers. As reported in #35, loading benchmark seems rigged because it uses a single process to test the init time of libraries, thus making the last one to test a clear winner.

In this PR I made it so the libraries are being tested via importing them in a separate node processes. It will take longer to test but the results seem to be more coherent. There is still some deviation but moving picocolors up and down does not make significant changes (comparing to the reported issue)

$ node benchmarks/loading.js
  chalk          4.687 ms
+ picocolors     0.225 ms
  cli-color     38.682 ms
  ansi-colors    1.056 ms
  kleur          1.641 ms
  kleur/colors   1.618 ms
  colorette      0.969 ms
  nanocolors     0.559 ms
$ node benchmarks/loading.js
+ picocolors     0.685 ms
  chalk          5.954 ms
  cli-color     40.953 ms
  ansi-colors    1.477 ms
  kleur          2.101 ms
  kleur/colors   1.444 ms
  colorette      1.077 ms
  nanocolors     0.704 ms
$ node benchmarks/loading.js
  chalk          5.189 ms
  cli-color     44.259 ms
  ansi-colors    1.162 ms
  kleur          1.619 ms
  kleur/colors   1.640 ms
  colorette      1.045 ms
  nanocolors     0.752 ms
+ picocolors     0.359 ms

cc @ai

Closes #35

@alexeyten
Copy link

Sometimes I've got negative loading time 🤣

Benchmarking is hard…

@alexeyten
Copy link

I've tested some loading using child_process.fork. Here some results

https://gist.github.com/alexeyten/667e6163777783ed02879a0e3e66dd5d

@alexeyraspopov
Copy link
Owner Author

@alexeyten, seems like your approach looks better, thank you! I'll try a couple of more tests and will update this PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Loading time benchmark is inaccurate
2 participants