Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Precompute point multiplication results in pedersen #380

Merged
merged 1 commit into from
Apr 14, 2023
Merged

Precompute point multiplication results in pedersen #380

merged 1 commit into from
Apr 14, 2023

Conversation

omerfirmak
Copy link
Contributor

Provides 5x performance improvement

@omerfirmak omerfirmak force-pushed the elim-pedersen-alloc branch 6 times, most recently from d55758f to 6fb8550 Compare April 12, 2023 18:36
@omerfirmak
Copy link
Contributor Author

@yelhousni, any chance we get an eye on this?

@yelhousni yelhousni self-requested a review April 13, 2023 10:27
@yelhousni yelhousni self-assigned this Apr 13, 2023
@yelhousni yelhousni added the perf label Apr 13, 2023
@yelhousni
Copy link
Collaborator

@yelhousni, any chance we get an eye on this?

So the point is instead of computing highPart*p2 + lowPart * p1 you precompute a table of multiples of p1 and p2 that you later lookup?

@omerfirmak
Copy link
Contributor Author

omerfirmak commented Apr 13, 2023 •
edited

@yelhousni, any chance we get an eye on this?

So the point is instead of computing highPart*p2 + lowPart * p1 you precompute a table of multiples of p1 and p2 that you later lookup?

Yeah, pretty much.

16 values are precomputed for each nibble in highPart and lowPart.

Then according to values of each nibble one of those values is selected per nibble and they are all summed up for the end result.

So we are replacing 2 scalar multiplications with multiple point additions.

@yelhousni
Copy link
Collaborator

Right! I remember proposing something in these lines: #327 (comment)

yelhousni
yelhousni approved these changes Apr 13, 2023
View reviewed changes
Copy link
Collaborator

@yelhousni yelhousni left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@omerfirmak
Copy link
Contributor Author

Thanks! Anything I can help with to get this merged?

@yelhousni yelhousni merged commit e500f2f into Consensys:develop Apr 14, 2023
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@yelhousni yelhousni yelhousni approved these changes

Assignees

@yelhousni yelhousni

Labels
perf
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@omerfirmak @yelhousni