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

Benchmark improvements #2169

Merged
merged 5 commits into from
Nov 24, 2023
Merged

Benchmark improvements #2169

merged 5 commits into from
Nov 24, 2023

Conversation

infogulch
Copy link
Contributor

@infogulch infogulch commented Nov 16, 2023

  • Remove accidentally committed files
  • CI: Clean offline; display cleaned bytes
  • Add missing csv bench to iai benchmarks
  • Generate flamegraphs for benchmarks
  • Validate benchmark outputs separately from criterion and iai runners
  • Incorporate feedback in benches docs
  • Upgrade iai to current HEAD to get new machine readable summary file output
  • Expose pub fn get_inference_count() on Machine
  • Save inference counts during benchmark validation as target/benchmark_inference_counts.json

@infogulch infogulch marked this pull request as draft November 16, 2023 21:46
@infogulch infogulch force-pushed the flame branch 2 times, most recently from d092841 to 2748536 Compare November 21, 2023 18:34
@infogulch infogulch changed the title Generate benchmark flamegraphs; misc cleanup Benchmark improvements Nov 21, 2023
* CI: Clean offline; display cleaned bytes; one artifact upload
* Add missing csv bench to iai benchmarks
* Generate flamegraphs for benchmarks
* Validate benchmark results separately
@infogulch
Copy link
Contributor Author

I think its ready, feedback notwithstanding.

With this all the data we need should be in json files in the benchmark-results artifact. A separate job/action should be able to collect the artifacts and publish charts etc.

@infogulch infogulch marked this pull request as ready for review November 22, 2023 00:24
@infogulch infogulch marked this pull request as draft November 22, 2023 03:51
@infogulch infogulch marked this pull request as ready for review November 22, 2023 04:49
@mthom
Copy link
Owner

mthom commented Nov 23, 2023

Looks good! Sorry for the conflicts I introduced. Will merge once resolved

Copy link
Contributor Author

@infogulch infogulch left a comment

Choose a reason for hiding this comment

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

👍

@mthom mthom merged commit f1458b7 into mthom:master Nov 24, 2023
13 checks passed
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