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

ci: Compare commit-comment benches by rc group #852

Merged

Conversation

samuelburnham
Copy link
Contributor

  • Enables the LURK_BENCH_OUTPUT="commit-comment" env var option, which configures the benchmark metadata for criterion-table formatting.
  • Moves the reduction count out of the BenchmarkId name and into the BenchmarkGroup, which separates bench run tables by rc in criterion-table.

@samuelburnham samuelburnham requested a review from a team as a code owner November 4, 2023 06:21
@samuelburnham samuelburnham marked this pull request as draft November 6, 2023 14:13
huitseeker
huitseeker previously approved these changes Nov 6, 2023
Copy link
Contributor

@huitseeker huitseeker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One nit: the fibo_prove / fibonacci_prove function naming doesn't hint at the benchmark organization. I'd rename fibo_prove to fibonacci_prove and call the old fibonacci_prove as fibonacci_prove_with_params or stg.

Differential CPU bench output for `LURK_RC="100,600" LURK_BENCH_OUTPUT="commit-comment"`

Benchmarks

Table of Contents

Benchmark Results

LEM Prove - rc=100

branch=ci-bench-comment-context branch=ci-bench-comment-context2
num-100 12.50 s (✅ 1.00x) 12.39 s (✅ 1.01x faster)
num-200 24.50 s (✅ 1.00x) 24.46 s (✅ 1.00x faster)

LEM Prove - rc=600

branch=ci-bench-comment-context branch=ci-bench-comment-context2
num-100 9.69 s (✅ 1.00x) 9.59 s (✅ 1.01x faster)
num-200 21.57 s (✅ 1.00x) 20.34 s (✅ 1.06x faster)

Made with criterion-table

@samuelburnham samuelburnham force-pushed the ci-bench-comment-context branch from 2a3a146 to 7549abf Compare November 6, 2023 15:36
@samuelburnham samuelburnham marked this pull request as ready for review November 6, 2023 15:40
@samuelburnham samuelburnham requested a review from a team as a code owner November 6, 2023 15:40
Copy link
Contributor

@huitseeker huitseeker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a bunch!

@samuelburnham samuelburnham added this pull request to the merge queue Nov 6, 2023
Merged via the queue into lurk-lab:master with commit ef96d64 Nov 6, 2023
25 of 28 checks passed
@samuelburnham samuelburnham deleted the ci-bench-comment-context branch November 6, 2023 16:53
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.

2 participants