-
-
Notifications
You must be signed in to change notification settings - Fork 419
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
Add before/after iteration hooks to ponybench #2898
Conversation
This commit adds `before_iteration` and `after_iteration` to ponybench benchmarks. It also changes the syncbenchmark `before` and `after` to allow them to throw errors. This commit also updates the `buffered` package benchmarks to use the new `before_iteration` and `after_iteration` hooks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One small leftover, otherwise lgtm!
packages/ponybench/_runner.pony
Outdated
|
||
be _complete_cont(e: U64) => | ||
be _after_done_cont(e: U64) => | ||
@printf[I32](("Done iterations: " + _n.string() + ", total time: " + _a.string() + "\n").cstring()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a debugging artifact, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ugh. Yes. I'll remove it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree that this falls under the principle of least surprise.
This commit adds
before_iteration
andafter_iteration
to ponybenchbenchmarks. It also changes the syncbenchmark
before
andafter
to allow them to throw errors.
This commit also updates the
buffered
package benchmarks to use thenew
before_iteration
andafter_iteration
hooks.NOTE: These changes might need to go through the RFC process. They seem like "principle of least surprise" to me but others may not agree.
old benchmark results:
new benchmark results:
The
*Samples
line items show how it looks using the current method to accomplish the same thing of doing before/after iteration setup/teardown via using1
iteration and1000000
samples. The equivalent new way of doing before/after iteration setup/teardown is the line immediately preceding the*Samples
line item. NOTE: the*Samples
benchmarks have been removed in the second commit of this PR and will not actually there after the PR is merged. They are here to show them as a contrast with the new approach.