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

Benchmarking using Criterion #20

Closed
wants to merge 4 commits into from
Closed

Conversation

Wasabi375
Copy link
Contributor

see #16

The files src/bin/01.rs, 02.rs, etc are still created by scaffold, but they now
call the solution in the src/day01.rs, src/day02.rs, thereby allowing benchmarking.

This means that scaffold also updates lib.rs to add the module definitions. Right now
this is done using a simple replace of "// pub mod day01;" with "pub mod day01;".
There is a probably a nicer way to do this, but it works.

Also due to an issue in cargo we need to run "cargo clean -p advent_of_code".
Otherwise "cargo check" will fail.
See rust-lang/cargo#6529
I forgot to add the comments after testing scaffolding
Don't hate on my bad implementation of the day 1 problems.
The solution for day 1 was my first time using rust.
@fspoettel
Copy link
Owner

fspoettel commented Dec 4, 2022

thank you for the contribution @Wasabi375. Let me know once this is in a reviewable state and I will take a look. I'm not entirely sure if I want to part with the bin-based setup as it's simple but being able to use criterion for benches is pretty tempting.

I'm very grateful to have a PoC to play around with here.

@Wasabi375
Copy link
Contributor Author

The setup I have right now is that the binaries for the different days still exist. They just call the solution that is implemented in day01.rs, etc. That way all the current stuff still works, but we move the solution into the lib so it can be run from criterion 😉

@fspoettel
Copy link
Owner

fspoettel commented Dec 4, 2022

I'm looking at the project with three lenses: 1

  • experience for people that are new to rust and not necessarily seasoned developers.
  • people who just want to do aoc in rust.
  • features & extensibility for more advanced users, myself included.

The current approach is (strongly) geared towards the former two. Having to deal with one isolated file every day and ignoring the complexity of the module system is a boon when starting with Rust. I used a module-based approach last year. I was pretty happy with the layout but still managed to shoot myself in the foot a bunch of times when my scaffolding functionality went haywire or I navigated myself into a weird build cache error. 🙈 From my point of view, the current approach also works well if the goal is to just solve advent of code.

That being said, binary crates have clear limitations, the biggest one them being black boxes that can't be imported from other rust code, as evident with implementing benchmarks. Any attempt on extending the template with more fancy features like writing benches to the readme will necessarily be limited in some way and somewhat hacky. Not impossible, but also not ideal.

Modules are more flexible here and IMO clearly better if you want to add more features like this to the template. If we adopt them proper, we would also not need the binary crates anymore - which are also one more file to understand if we keep them around. So the main overhead that stays is dealing with the module system, e.g. when registering and using the modules.

The question, for me, is if the (clean) implementation of this feature warrants an IMO higher barrier of entry to using the template.

Footnotes

  1. I also do not fully understand who is using this template right now as it got a bit more usage than I anticipated when creating it. 😅

@fspoettel fspoettel mentioned this pull request Dec 6, 2022
@Wasabi375
Copy link
Contributor Author

I had some personal stuff pop up in the last few days so I've been really busy. I don't think I will have the time to finish working on this. Also looking back I'm not sure I had the best aproach to solve this here. As I said, I just started to learn rust and I'm sure there a better ways to solve this than the hack I started to implement here.
With that said I will close this Draft here. If anyone wants to use the code I have so far, feel free, although I would probably advice against it.
Maybe I will revisit this in the future. Either when I need a nice playground to learn about criterion or maybe next year for AoC.

@Wasabi375 Wasabi375 closed this Dec 18, 2022
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