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

bench: switch to criterion #149

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

jcdickinson
Copy link

This is a "PR that could have been an issue," I'm opening this PR to determine if this is a direction that the project wants to take.

Switches the benchmark framework to Criterion. The default Rust std test framework has a hardcoded goal execution time of 1ms for each benchmark. This makes it unsuitable for sample-based profiling because the benchmark doesn't run for very long.

Adds a utility script to run tests under a profiler.

Switches the benchmark framework to Criterion. The default Rust std test
framework has a hardcoded goal execution time of 1ms for each benchmark.
This makes it unsuitable for sample-based profiling because the
benchmark doesn't run for very long.

Adds a utility script to run tests under a profiler.
@RazrFalcon
Copy link
Collaborator

I'm aware of criterion, but it's an overkill for our use case. And it's a very heavy dependency. The current implementation is more than enough.

@jcdickinson
Copy link
Author

The current implementation is more than enough.

@RazrFalcon what about the profiling issue I mentioned in the description? 1ms of sampling is certainly completely inadequate to create a high quality profile: https://github.com/rust-lang/rust/blob/master/library/test/src/bench.rs#L132

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