-
Notifications
You must be signed in to change notification settings - Fork 37
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
perf: Profile and fix slow zero-initialization #283
Conversation
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.
I'm seeing some worrying preliminary results with these changes. a big slowdown. Let's make sure we have a benchmark with relevant set of parameters (for recursive verification) on our current reference machine before we merge.
With the memory warmup, that I added in my latest commit to this branch, this PR gives a speedup of about 2kHz on a canonical parameter set on our benchmark machine. Current master
This PR
|
The line you added also makes it glaringly obvious that every element is in fact written to before it is read from, which I like tremendously. |
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.
Maybe add comment about why memory warmup is needed? See e.g. commit message for 21cc30e
Otherwise happy.
21cc30e
to
25670f1
Compare
This fix uses the unsafe keyword, but Thorkil is (speaking as "we" are) convinced that this is safe because all elements are being written to before read from. Co-authored-by: Thorkil Værge <[email protected]>
25670f1
to
f7b13e7
Compare
This fix uses the unsafe keyword, but we are convinced that this is safe because all elements are being written to before read from.