Closed nph4rd closed 3 years ago
@tholop - attending your comments, I've grouped the benchmarks with ranges of n_values
and n_threads
. I'm not sure how this will play out in the GitHub Actions runners (see info here) but hopefully this is closer to what will be useful for us. Let me know if there's anything missing.
Looks perfect, thanks @arturomf94! I realized that the speed was the same regardless of the value of n_threads
-- not because of the benchmark code. The reason is that if a Rayon thread pool already exists, Sycret will not initialize a new one: https://github.com/OpenMined/sycret/blob/fc776e1a9c517aebdd7a705d448eb1865cda6479/src/lib.rs#L84
Most of the time, the user will not provide any value for n_threads
anyway, so Sycret will directly use the right number of threads, but this is not very clean. We don't want to pay the Rayon pool initialization overhead at every call, but I think that we can respawn the pool only when n_threads
is different than rayon::current_num_threads
. What do you think?
I'm happy to push a commit for this change.
We don't want to pay the Rayon pool initialization overhead at every call, but I think that we can respawn the pool only when n_threads is different than rayon::current_num_threads
Yes - I think this sounds optimal.
Description
We've already got some benchmarks but we're missing one for batch gen/eval. This PR adds that.
Affected Dependencies
.
How has this been tested?
Running benchmarks locally.
Checklist