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

Feature request: automatic warmup #82

Open
colinhacks opened this issue May 15, 2024 · 1 comment
Open

Feature request: automatic warmup #82

colinhacks opened this issue May 15, 2024 · 1 comment

Comments

@colinhacks
Copy link

colinhacks commented May 15, 2024

It seems a little odd that you must call .warmup() even if you've already specified warmup* options when initializing the benchmark. I respect the choice to make warmups explicit, but I think there may be an API that accomplishes both.

The idea is to add a warmup key to Options that can accept boolean | { time?: number; iterations?: number }. If this key is specified & truthy, the benchmark will automatically call .warmup() when run.

const a = new Bench({ warmup: true }); // auto-warmup, default config
const b = new Bench({ warmup: { time: 400 }}); // auto-warmup, warmupTime=400
const b = new Bench({ warmup: { iterations: 1000 }}); // auto-warmup, iterations=400

This is a non-breaking change since it's a brand new option. Users opt into the "auto warmup" feature by specifying warmup. The existing keys warmupTime and warmupIterations can continue to be supported though you'd presumably throw an informative error if both warmup.time and warmupTime were specified.

Just a thought. Happy to put in a PR if this seems acceptable. Thanks for the great tool!

@Aslemammad
Copy link
Member

This seems pretty cool! In the next major version, this would also help removing the .warmup method! Let's go with it.

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

No branches or pull requests

2 participants