-
Notifications
You must be signed in to change notification settings - Fork 44
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 for Nullable #24
Conversation
I would use the |
|
||
const SUITE = BenchmarkGroup() | ||
|
||
srand(1) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Like @KristofferC mentioned, you can do using .RandUtils
and then replace uses of rand
in this module with samerand
.
Thanks for the contribution, the more benchmarks the merrier! Since the benchmark parameters here don't yet exist in I should make that method less fragile...let me fix that, then you can rebase this PR onto the fix so that tests here pass. EDIT: Done with the fix. Feel free to rebase this against master and see if the tests pass. |
Thanks! I've just rebased against master, used I'd still like to hear @johnmyleswhite and @DavidGold's comment about these. FWIW, I've just checked that |
7fa580c
to
9b21364
Compare
Oh, and it is fine/recommended to test so many types in a loop like I do? If that takes too much time, I can keep only a subset of them. |
1c5d318
to
1f8452b
Compare
Thanks for doing this, Milan. Will this be coordinated with JuliaLang/julia#18510 ? I can think of more benchmarks for |
The idea was that if we merge these benchmarks first, they will allow checking that the move to
Could you give an example of what you mean? Is that just something similar to |
Should be fine. The majority of turnaround time for Nanosoldier jobs is actually building Julia rather than running benchmarks.
Just making sure you're aware - benchmarks merged here won't immediately be available via Nanosoldier, as they have to undergo a tuning process (which I can trigger tomorrow, if this PR is merged by then). Other than triggering the process and committing the results, the tuning process is pretty automated. When I get the chance, I should probably set up a cron job that does this nightly... |
Also, let me know once you're ready for me to merge this and I'll do so |
1f8452b
to
20dedf9
Compare
Thanks. Should be good to merge now. It would be nice if you could run it on Nanosoldier, as it would make a reference point for current PRs against Base. |
Includes in particular a reduced version of NullableArrays to measure loop performance.
20dedf9
to
7506c1a
Compare
Includes in particular a reduced version of NullableArrays to measure loop
performance.
This is my first experience with this package, so please give my advice on what benchmarks should look like.