Optimize Msm #796
Merged
Optimize Msm #796
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
str4d
approved these changes
Dec 4, 2025
str4d
requested changes
Dec 4, 2025
str4d
force-pushed
the
feature/msm-optimization
branch
from
d897fd4 to
7de8af3
Compare
str4d
approved these changes
Dec 4, 2025
str4d
reviewed
Dec 4, 2025
halo2_proofs/src/arithmetic.rs
Outdated
| let mut acc = C::Curve::identity(); | ||
| let mut sum = C::Curve::identity(); | ||
| buckets.iter().rev().for_each(|b| { | ||
| sum = b.add(sum); |
Contributor
There was a problem hiding this comment.
That doesn't work, because sum has type C::Curve, not Bucket, and we can't impl<C: Curve> AddAssign<Bucket<C::AffineRepr>> for C because it would be a foreign impl.
EDIT: I now see you edited your suggestion, and that one works fine.
str4d
reviewed
Dec 4, 2025
| } | ||
| Bucket::Projective(a) => other + &a, | ||
| } | ||
| fn add(self, mut other: C::Curve) -> C::Curve { |
Contributor
There was a problem hiding this comment.
Bucket::add was an existing function that is preserved in this PR, so in the interest of not needing to go through another round of CI, I'm going to defer this internal rename.
nuttycom
approved these changes
Dec 4, 2025
| type Item; | ||
|
|
||
| /// Combines the best of `std::iter` and `rayon` reductions. | ||
| fn the_best_reduce( |
Contributor
There was a problem hiding this comment.
🙃 at the name, but this is fine.
Contributor
There was a problem hiding this comment.
I needed something that was not likely to clash in future with a std or rayon name 😄
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Msm Optimization
Hi there.
I optimized the best_multiexp algorithm and replaced it with a more efficient rayon method.
What I did
Task Size
Before
Current implementation divides task into coeffs length / thread number parts.
The greater the parallelism used by
rayon(by default given by the number of logical cores as determined by std::thread::available_parallelism), the smaller the task size of each thread process.It causes task size to be too heavy for small-core PCs and too small for large-core PCs.
After
Flatten task size to addition and sum of bucket for each segment.
Parallelization Scope
Before
The current implementation performs final sum with a non-parallel process.
After
Include the final sum to parallel scope.
Rayon Method
Before
According to official documentation, speed relation is as follows: join ≤ par_iter ≤ scope
After
Replace
best_multiexpscope withpar_iterOther
Refactoring
Bucketandget_at.Benchmark
I ran best_multiexp benchmark twice
after -> beforeandbefore -> afterin order to make machine condition same.First
Second
I would appreciate it if you could confirm.
Thank you.