-
Notifications
You must be signed in to change notification settings - Fork 231
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: optimize proof generation #509
Conversation
aeb4754
to
446b215
Compare
c69c990: parallel loops -> loops? |
I said "merge parallel loops" because |
4807e85
to
0018824
Compare
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.
LGTM
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.
79c9de0
Please change the commit body. I’m not confident my change accurately depicts the changes. Please check that it does!
Removes code duplicates and optimizes potential parallelization.
If the input of BatchNormalize()
is large enough, vector creation is parallelized. Else, the
operation is run serially, reducing the number of allocations.
4669aec
Change first line of body to “merge parallel loops since F::GetSuccessivePowers()
uses parallel loops independently.” ("use" -> "uses")
715db8b
Please change the commit body as such:
Reduces the number of thread joins and heap allocations.
Additionally fixes bugs in GetSelectorsOnCoset()
replacing
chunk_size
with len
.
fb2ea95
Commit title-> “prevent possible overflow error”
0018824
Commit title-> “fix ScalarMul
result of multiplying by zero”
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.
LGTM
379563f
to
c34c360
Compare
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.
LGTM
tachyon/math/elliptic_curves/msm/algorithms/pippenger/pippenger_adapter.h
Outdated
Show resolved
Hide resolved
c34c360
to
d2b6169
Compare
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.
LGTM
d2b6169
to
bd0aee8
Compare
- use `std::move()` where possible - release intermediate vector immediately - reserve `evals` size
Removes code duplication and optimizes potential parallelization. Previously, when running serially, allocations occurred but were not used. Additionally, when running with parallelization, the allocation was not parallelized. Now, if the input to `BatchNormalize()` is large enough, vector creation is parallelized properly and proper allocation usage is ensured.
- merge parallel loops since `F::GetSuccessivePowers()` uses parallel loops independently. - use `std::move()` where possible
This is needed to construct `BigInt` with `Eigen::Index` from macOS.
Reduces the number of thread joins and heap allocations. Additionally, fixes bugs in `GetSelectorsOnCoset()` replacing `chunk_size` with `len`.
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.
LGTM
bd0aee8
to
6b5b8a7
Compare
Description
This PR optimizes parallel loops. (Including refactoring + bugfix)