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

Benchmarks #113

Draft
wants to merge 10 commits into
base: master
Choose a base branch
from
Draft

Benchmarks #113

wants to merge 10 commits into from

Conversation

dlesnoff
Copy link
Contributor

I benchmarked multiplication, division, random generation of bigints. Without flags, the operations are slow, with flags, the timings looks correct to me.
The code source quality of benchBigints.nim can be improved with templates to do less copy-paste in the future.
I have also changed some of the examples to import them (by exporting some procedures and using whenIsmainmodule) so that I can benchmark them.
I added some corresponding nimble tasks.
I use -d:lto -d:danger --opt:speed flags for benchmarks.
I consider using benchy for benchmarks is ok, it does not make it a dependency of the full library, it is only required for benchmarks.
This PR might be really useful for easy comparison of our changes to the library.

Copy link
Contributor

@konsumlamm konsumlamm left a comment

Choose a reason for hiding this comment

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

Generally looks good to me! I'm not really fond of benchy, since it only displays numbers in milliseconds, but I don't know of a better alternative. The benchmarking code has a lot of duplication, that could be easily reduced by extracting the benchmarking code into a proc/template. I think we should also bench BigInts with less than 100 limbs, e.g. 10, 2, or 1, since I think those will be more common for casual uses of BigInts.

Comment on lines +790 to +796
# block:
# randomize()
# let a: BigInt = initRandomBigInt(33)
# echo a

static: main()

#static: main()
Copy link
Contributor Author

@dlesnoff dlesnoff Aug 15, 2022

Choose a reason for hiding this comment

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

I had to comment out the static tests and randomized versions. In the future, I will need to remove these comments and let this file in its initial state.

@pietroppeter
Copy link
Contributor

pietroppeter commented Aug 19, 2022

in the past I did some benchmarking (addition and multiplication, inspired by a Js benchmarking suite) during a PR (then removed the code). They were done using library criterion and published results in a generated md file. Maybe it can turn out useful for this PR: #35 (comment)

@dlesnoff dlesnoff mentioned this pull request May 18, 2023
@dlesnoff dlesnoff marked this pull request as draft May 18, 2023 10:28
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