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 benchmarks #43

Closed
wants to merge 8 commits into from
Closed

Add benchmarks #43

wants to merge 8 commits into from

Conversation

NickCondron
Copy link
Contributor

@NickCondron NickCondron commented Dec 27, 2022

achieves #38

The benchmark I setup runs all replays in benches/data/ for each of three test cases (7 replays * 3 = 21 total benchmarks):

  1. Parse into a Game struct
  2. Parse by passing in a dummy Handlers struct
  3. Parse into a Game struct with skip_frames: true

Note: currently a bug prevents peppi from parsing the entire casual_doubles.slp replay (see #42). However, the benchmark still runs fine.

Currently the benchmarks times are inconsistent (roughly +/- 5% or so) on my laptop. I tried playing with the settings, but couldn't fully shake the inconsistency. Maybe a different computer would do better. The replays are setup where reading from disk and dropping the Game does not affect the results, but other factors like system load can affect things. This is why I'm considering using iai also.

Edit: I added Iai benchmarks and they look great. Run-to-run they change <1% except sometimes for the skip_frames tests (but those are so much faster anyways it's not a big concern). We should keep both though because they each have their pros and cons.

@NickCondron NickCondron marked this pull request as draft December 28, 2022 13:04
@NickCondron NickCondron force-pushed the benchmarking branch 2 times, most recently from 67e98c6 to a30dcd2 Compare December 29, 2022 07:47
@NickCondron NickCondron marked this pull request as ready for review December 29, 2022 07:47
@hohav
Copy link
Owner

hohav commented Dec 9, 2023

Merged via b62b811 and 0c671c8. Thank you!

@hohav hohav closed this Dec 9, 2023
@hohav hohav mentioned this pull request Dec 27, 2023
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