[RFC] CI for running Benchmarks

[YJDoc2]

Given that there are quite a few enhancements requested (#291 and linked in its desc), there will be code changes that potentially change the processing path or structures to implement them. As this is a parsing library and has speed as a concern, there should be a way to track change in processing time along with commits.

Currently I think there are benchmarks in makefile, so we should probably allow running them on github itself through CI. One way can be to run on each push, other would be to run through comment.

Would a PR on this welcome? Any suggestions on how to do benchmarks apart from the makefile and/or approach?

Thanks!

[gjtorikian]

I think this is a good improvement. I would suggest that the action run once on the pull_request_target: opened event, and also, listen for comments. This way every new PR can have a quick gut check on how perf is affected; and if it turns out that on-demand runs need to occur, they can. Running on every push would be to expensive (literally and computationally) and noisy; and running on just comments might resulting in "hiding" the information unless you know how to run it. A PR comment that presents the benchmarks and then tells you how to run the comment command would be a nice balance.

[YJDoc2]

I think that makes sense! We can have an initial run when PR is opened, and then ad-hoc runs through comments. The reporting comment can be sticky, so there is not too much noise, and as you suggest it can also mention the way to re-run and update.

I see that the bench in makefile o/p gives mean, median and stdev ; so should we report these raw and diff from master or just the diff? Also should we also report the values for cmark-gfm to compare?

[gjtorikian]

so should we report these raw and diff from master or just the diff?

I think raw + diff makes sense here. If it's helpful, here's an example of running the complete program (though it sounds like you know where to look already :)).

Also should we also report the values for cmark-gfm to compare?

I think cmark-gfm plus the two libraries mentioned in the README ([pulldown-cmark](https://github.com/google/pulldown-cmark) and markdown-rs) would be nice.

[kivikakk]

I think such a change is a really good idea! Very much welcome.

My main concern is about measurement noise from the execution environment? The benchmark script uses the user-space time exclusively (and not real/wall-clock), which should exclude variance due to vCPU scheduling etc. in VMs like those used by Actions, so it's likely fine.

The reporting comment can be sticky, so there is not too much noise, and as you suggest it can also mention the way to re-run and update.

This is very neat!

[YJDoc2]

Great! I'll try to get a PR up this weekend.

Also what are your thoughts on owning the benchmarking script, instead of using that of cmark-gfm? We will still be using same approach of using the pro-git md files, but instead of just time we can use something else (maybe hyperfine?) to run the benchmarks, so we can maybe have better measurements and have o/p format as we need.

This can be split into a separate PR, so we can just do the CI update first and then think on changing the bench script, but wanted to take comments on this as well.

[digitalmoksha]

CI for running Benchmarks

I think this is a great idea, I've had something similar in the back of head for awhile.

[kivikakk]

Also what are your thoughts on owning the benchmarking script, instead of using that of cmark-gfm? We will still be using same approach of using the pro-git md files, but instead of just time we can use something else (maybe hyperfine?) to run the benchmarks, so we can maybe have better measurements and have o/p format as we need.

This can be split into a separate PR, so we can just do the CI update first and then think on changing the bench script, but wanted to take comments on this as well.

This is completely fine, and I don't think you need to split up the PRs unless you really want to; there's nothing very special about their method, and if we show comparisons against cmark-gfm with our method, there's nothing lost!

[YJDoc2]

I proposed splitting in case you wanted to have that evaluated separately (as they are two changes - adding ci and changing bench). Anyways, I'll do both in same PR then 👍

To be clear, I'm proposing following changes for benches :

• I'll be adding pulldown cmark and markdown rs as submodules
• basically copy the bench script from cmark-gfm to clone pro-git and run benches
• try out replacing time command with hyperfine or something similar, which can provide options such as cache dropping and memory usage measures
• add two targets in comrak's makefile - bench and bench-all, where we will run bench for just comrak or all three
• Do necessary plumbing to make all this work and provide a nice o/p
• Add CI as suggested in previous comments.

[kivikakk]

Sounds perfect. 👍 Thank you! 🤍