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

Add before/after iteration hooks to ponybench #2898

Merged
merged 3 commits into from
Oct 18, 2018

Conversation

dipinhora
Copy link
Contributor

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.


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:

vagrant@ubuntu-xenial:~/dhp/packages/buffered/benchmarks$ ./benchmarks.after --ponythreads=1 --ponyminthreads=1
Benchmark results will have their mean and median adjusted for overhead.
You may disable this with --noadjust.

Benchmark                                   mean            median   deviation  iterations
_ReaderU8                                  34 ns             34 ns      ±2.76%     2000000
_ReaderU16LE                               33 ns             34 ns      ±1.62%     2000000
_ReaderU16LESplit                         459 ns            493 ns     ±14.70%     1000000
_ReaderU16BE                               43 ns             43 ns      ±4.41%     2000000
_ReaderU16BESplit                         502 ns            513 ns      ±8.88%     1000000
_ReaderU32LE                               45 ns             45 ns      ±1.58%     2000000
_ReaderU32LESplit                         925 ns            944 ns      ±6.64%      300000
_ReaderU32BE                               45 ns             45 ns      ±2.91%     2000000
_ReaderU32BESplit                        1022 ns           1040 ns      ±7.33%      500000
_ReaderU64LE                               49 ns             49 ns      ±2.69%     2000000
_ReaderU64LESplit                        1841 ns           1878 ns      ±8.32%      100000
_ReaderU64BE                               49 ns             49 ns      ±2.64%     2000000
_ReaderU64BESplit                        1934 ns           1919 ns      ±5.91%      100000
_ReaderU128LE                              51 ns             50 ns      ±2.20%     2000000
_ReaderU128LESplit                       3812 ns           3839 ns      ±7.86%       50000
_ReaderU128BE                              40 ns             40 ns      ±1.05%     2000000
_ReaderU128BESplit                       3055 ns           3252 ns     ±17.97%       50000
_WriterU8                                   8 ns              8 ns      ±1.28%     3000000
_WriterU16LE                                5 ns              5 ns      ±1.14%     5000000
_WriterU16BE                                7 ns              7 ns      ±1.47%     5000000
_WriterU32LE                                6 ns              6 ns      ±1.70%     3000000
_WriterU32BE                                7 ns              6 ns      ±1.61%     3000000
_WriterU64LE                                8 ns              8 ns      ±1.55%     3000000
_WriterU64BE                                7 ns              7 ns      ±1.73%     3000000
_WriterU128LE                               7 ns              7 ns      ±0.65%     3000000
_WriterU128BE                              10 ns             10 ns      ±0.98%     2000000

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 using 1 iteration and 1000000 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.

vagrant@ubuntu-xenial:~/dhp$ ./benchmarks --ponythreads=1 --ponyminthreads=1
Benchmark results will have their mean and median adjusted for overhead.
You may disable this with --noadjust.

Benchmark                                   mean            median   deviation  iterations
_ReaderU8                                  43 ns             42 ns      ±2.04%     2000000
_ReaderU8Samples                           47 ns             42 ns    ±438.12%           1
_ReaderU16LE                               47 ns             45 ns      ±9.94%     2000000
_ReaderU16LESamples                        44 ns             44 ns    ±345.14%           1
_ReaderU16LESplit                          76 ns             76 ns      ±1.20%     1000000
_ReaderU16LESplitSamples                   91 ns             78 ns    ±492.33%           1
_ReaderU16BE                               45 ns             44 ns      ±2.45%     2000000
_ReaderU16BESplit                          75 ns             75 ns      ±2.49%     1000000
_ReaderU32LE                               41 ns             41 ns      ±1.27%     2000000
_ReaderU32LESplit                         131 ns            130 ns      ±1.97%     1000000
_ReaderU32BE                               43 ns             43 ns      ±0.93%     2000000
_ReaderU32BESplit                         120 ns            120 ns      ±1.20%     1000000
_ReaderU64LE                               39 ns             39 ns      ±1.13%     2000000
_ReaderU64LESplit                         221 ns            220 ns      ±1.19%      500000
_ReaderU64BE                               35 ns             35 ns      ±1.07%     2000000
_ReaderU64BESplit                         222 ns            221 ns      ±1.42%      500000
_ReaderU128LE                              33 ns             33 ns      ±1.42%     2000000
_ReaderU128LESplit                        398 ns            397 ns      ±1.67%      300000
_ReaderU128BE                              42 ns             40 ns      ±8.17%     2000000
_ReaderU128BESplit                        424 ns            423 ns      ±1.60%      300000
_WriterU8                                   3 ns              3 ns      ±1.21%     5000000
_WriterU16LE                                2 ns              2 ns      ±1.52%     5000000
_WriterU16BE                                3 ns              3 ns      ±1.73%     5000000
_WriterU32LE                                2 ns              2 ns      ±2.14%     5000000
_WriterU32BE                                3 ns              3 ns      ±1.00%     5000000
_WriterU64LE                                3 ns              3 ns      ±1.59%     5000000
_WriterU64BE                                3 ns              3 ns      ±0.82%     5000000
_WriterU128LE                               6 ns              6 ns      ±0.80%     3000000
_WriterU128BE                               5 ns              5 ns      ±4.55%     3000000

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.
Copy link
Contributor

@mfelsche mfelsche left a 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!


be _complete_cont(e: U64) =>
be _after_done_cont(e: U64) =>
@printf[I32](("Done iterations: " + _n.string() + ", total time: " + _a.string() + "\n").cstring())
Copy link
Contributor

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?

Copy link
Contributor Author

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.

Copy link
Contributor

@Theodus Theodus left a 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.

@Theodus Theodus added the changelog - changed Automatically add "Changed" CHANGELOG entry on merge label Oct 18, 2018
@Theodus Theodus merged commit 5f68a5a into ponylang:master Oct 18, 2018
ponylang-main added a commit that referenced this pull request Oct 18, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog - changed Automatically add "Changed" CHANGELOG entry on merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants