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

feat: add ec benchmark #147

Merged
merged 17 commits into from
Jul 6, 2022
Merged

feat: add ec benchmark #147

merged 17 commits into from
Jul 6, 2022

Conversation

BoyuanFeng
Copy link
Contributor

@BoyuanFeng BoyuanFeng commented Jun 30, 2022 •
edited
Loading

Summary

Add benchmarks for Arkworks elliptic curve operations.

Raw latency

CPU: @11th Gen Intel(R) Core(TM) i5-1135G7 @ 2.40GHz

For a single point:

  • affine-affine addition: 12.4 us (this is slower than projective-projective addition since it includes an into_affine())
  • projective-affine addition: 1.53 us
  • projective-projective addition: 1.61 us
  • affine-scalar multiplication: 347.5 us
  • projective-scalar multiplication: 433.7 us
  • projective to affine normalization: 9.3 us

For a vector of $2^{16}$ points:

  • batch projective to affine normalization: 43.893 ms
  • naive projective to affine normalization (i.e. calling projective to affine normalization on individual elements): 1.17 s

Closes: #135

Before we can merge this PR, please make sure that all the following items have been checked off:

  • Linked to an issue with discussion and accepted design OR have an explanation in the PR that describes this work.
  • Added one line describing your change in CHANGELOG.md and added the appropriate changelog label to the PR.
  • Re-reviewed Files changed in the GitHub PR explorer.
  • Checked that changes and commits conform to the standards outlined in CONTRIBUTING.md.

@BoyuanFeng BoyuanFeng added the changelog:added Changelog: add these changes to the `added` section of the changelog label Jun 30, 2022
@BoyuanFeng BoyuanFeng self-assigned this Jun 30, 2022
@BoyuanFeng BoyuanFeng added this to the v0.5.3 milestone Jun 30, 2022
@BoyuanFeng BoyuanFeng requested review from bhgomes and tsunrise and removed request for bhgomes June 30, 2022 22:15
bhgomes
bhgomes suggested changes Jul 1, 2022
View reviewed changes
Copy link
Contributor

@bhgomes bhgomes left a comment

Choose a reason for hiding this comment

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

Also, all the functions in the ec module should be marked #[inline].

manta-benchmark/src/ec.rs Outdated Show resolved Hide resolved
manta-benchmark/src/ec.rs Outdated Show resolved Hide resolved
manta-benchmark/src/ec.rs Outdated Show resolved Hide resolved
manta-benchmark/src/ec.rs Outdated Show resolved Hide resolved
manta-benchmark/benches/ec.rs Outdated Show resolved Hide resolved
Boyuan Feng added 2 commits July 1, 2022 10:56
feat: nit
b906b36
Merge branch 'main' into feat/ec-benchmark
886761b 
Signed-off-by: Boyuan Feng <bfeng9@wisc.edu>
tsunrise
tsunrise suggested changes Jul 1, 2022
View reviewed changes
Copy link
Contributor

@tsunrise tsunrise left a comment

Choose a reason for hiding this comment

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

Nice work! Minor comments

manta-benchmark/src/ec.rs Outdated Show resolved Hide resolved
manta-benchmark/benches/ec.rs Outdated Show resolved Hide resolved
manta-benchmark/benches/ec.rs Outdated Show resolved Hide resolved
manta-benchmark/benches/ec.rs Outdated Show resolved Hide resolved
@tsunrise tsunrise mentioned this pull request Jul 1, 2022
Open
@bhgomes bhgomes modified the milestones: v0.5.3, v0.5.4, v0.6.0 Jul 1, 2022
@bhgomes bhgomes modified the milestones: v0.6.0, v0.6.1 Jul 4, 2022
@bhgomes bhgomes added A-performance Area: Performance-related Issues and PRs and removed norelease labels Jul 6, 2022
@bhgomes bhgomes added the P-medium Priority: Medium label Jul 6, 2022
@BoyuanFeng BoyuanFeng requested review from bhgomes and tsunrise July 6, 2022 19:57
Boyuan Feng and others added 2 commits July 6, 2022 15:58
chore: fix a lint issue
e9aec35
@bhgomes
fix: resolve inconsistencies in benchmarking suite
112dbdf 
- Rename `ec` to `ecc`
- Add missing documentation lints
- Use consistent `black_box` and let-binding drop rules for all `ecc`
  benchmarks
- Fix `Cargo.toml` issues

Signed-off-by: Brandon H. Gomes <bhgomes@pm.me>
bhgomes
bhgomes suggested changes Jul 6, 2022
View reviewed changes
Copy link
Contributor

@bhgomes bhgomes left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Almost done. @BoyuanFeng please add basic documentation to all the code including modules, structs, and functions (including benchmarks + tests). Also, @tsunrise please check if the black_box rules are being followed correctly in the mint, private_transfer, and reclaim benchmarks.

@BoyuanFeng BoyuanFeng requested a review from bhgomes July 6, 2022 21:52
@bhgomes bhgomes merged commit e36a0dc into main Jul 6, 2022
@bhgomes bhgomes deleted the feat/ec-benchmark branch July 6, 2022 22:48
@bhgomes bhgomes modified the milestones: v0.6.1, v0.5.5 Aug 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tsunrise tsunrise tsunrise approved these changes

@bhgomes bhgomes bhgomes approved these changes

Assignees

@BoyuanFeng BoyuanFeng

Labels
A-performance Area: Performance-related Issues and PRs changelog:added Changelog: add these changes to the `added` section of the changelog P-medium Priority: Medium
Projects
None yet
Milestone
v0.5.5
Development

Successfully merging this pull request may close these issues.

Add Benchmarks for Arkworks Elliptic Curve Operations
3 participants
@BoyuanFeng @bhgomes @tsunrise