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

dec: Expand benchmarks #56

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

jean-airoldie
Copy link
Contributor

This expands the benchmarks for the various number types. For the choice of operations, I used what was already benchmarked and I added functions I expect to be commonly used. Feel free to suggest anything else that could be relevent. I also wasn't too sure what sizes of Decimal<T> to test for.

One problem with the decimal generation approach I used is that the numbers are totally random and thus don't really follow any real world statistical distribution. This shouldn't be a big problem as long as the benchmark are only used to gauge relative improvement in performance etc.

@CLAassistant
Copy link

CLAassistant commented Oct 16, 2021

CLA assistant check
All committers have signed the CLA.

@jean-airoldie jean-airoldie changed the title Expand benchmarks dec: Expand benchmarks Oct 16, 2021
* Add benchmarks for basic ops (add, sub, mul, div, etc.) for the
  decimal types.
* Use seeded rng for reproducible results.
* Use a longer measurement period for more consistant results.
Copy link
Contributor

@sploiselle sploiselle left a comment

Choose a reason for hiding this comment

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

Appreciate this expansion/tidying up. 12 and 16 are reasonable values to test Decimal<N> for so 👍 . No other suggestions for functions to add, but this makes further extensions much cleaner.

LGTM modulo unnesting imports.

@jean-airoldie
Copy link
Contributor Author

LGTM modulo unnesting imports.

I'm not exactly sure of what that means. I'm guessing you mean that you prefer a single import group per line.

@sploiselle
Copy link
Contributor

@jean-airoldie sorry about that, seems like my comment about the unnested imports didn't propagate as expected. You correctly inferred my meaning, though. tysm.

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.

3 participants