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.

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

Refactor MSM instances in the IPA #290

Merged
merged 5 commits into from
Feb 2, 2024
Merged

Conversation

huitseeker
Copy link
Member

@huitseeker huitseeker commented Feb 1, 2024

What's in this PR

This experiments with re-structuring the IPA to:

  • avoid implicit or needless CommitmentKey clones (I've had to import some changes from Remove superfluous instances of derive(Clone) (easy) #287),
  • avoid depending on calling the MSM in impl CommitmentKeyExtTrait for pedersen::CommitmentKey, instead relying on an already-available implementation of Mul<E::Scalar, Output = E> for E::Affine.

That experiement is largely unsuccessful, since MSM implementation (which resolves to cpu_best_msm since the scalar length is 2) is about 5% faster1. We tactically revert to the MSM in that particular line.

Benchmarks

I ran the pcs benchmarks (click to unfold)
group                                                                                                                                      dev                                unmsm_ipa
PCS-Proving 10/arecibo::provider::ipa_pc::EvaluationEngine<arecibo::provider::Bn256Engine>/10                                              1.00     32.2±0.18ms    1.00     32.1±0.28ms
PCS-Proving 12/arecibo::provider::ipa_pc::EvaluationEngine<arecibo::provider::Bn256Engine>/12                                              1.00     86.3±0.35ms    1.00     86.7±0.22ms
PCS-Proving 14/arecibo::provider::ipa_pc::EvaluationEngine<arecibo::provider::Bn256Engine>/14                                              1.00    295.3±1.40ms    1.00    294.1±1.19ms
PCS-Proving 16/arecibo::provider::ipa_pc::EvaluationEngine<arecibo::provider::Bn256Engine>/16                                              1.00   1115.9±5.35ms    1.00   1114.3±5.28ms
PCS-Proving 18/arecibo::provider::ipa_pc::EvaluationEngine<arecibo::provider::Bn256Engine>/18                                              1.00       4.4±0.04s    1.00       4.4±0.03s
PCS-Proving 20/arecibo::provider::ipa_pc::EvaluationEngine<arecibo::provider::Bn256Engine>/20                                              1.00      17.5±0.07s    1.00      17.5±0.05s
PCS-Verifying 10/arecibo::provider::ipa_pc::EvaluationEngine<arecibo::provider::Bn256Engine>/10                                            1.04      3.4±0.05ms    1.00      3.3±0.04ms
PCS-Verifying 12/arecibo::provider::ipa_pc::EvaluationEngine<arecibo::provider::Bn256Engine>/12                                            1.04      5.8±0.10ms    1.00      5.6±0.17ms
PCS-Verifying 14/arecibo::provider::ipa_pc::EvaluationEngine<arecibo::provider::Bn256Engine>/14                                            1.00     13.9±0.37ms    1.01     14.0±0.43ms
PCS-Verifying 16/arecibo::provider::ipa_pc::EvaluationEngine<arecibo::provider::Bn256Engine>/16                                            1.01     35.9±0.79ms    1.00     35.6±0.26ms
PCS-Verifying 18/arecibo::provider::ipa_pc::EvaluationEngine<arecibo::provider::Bn256Engine>/18                                            1.00    124.9±0.95ms    1.00    124.8±0.84ms
PCS-Verifying 20/arecibo::provider::ipa_pc::EvaluationEngine<arecibo::provider::Bn256Engine>/20                                            1.00   506.0±30.68ms    1.01   508.9±21.72ms

Note

Companion PR at lurk-lab/lurk-rs#1088

Footnotes

  1. note the scale function moving away from the MSM doesn't seem to have nearly the same perf. impact,

- Added new trait usage and extended `AffineExt` with more bounds in `src/provider/traits.rs` to enable greater functionality.
- new bounds on AffineExt are trivially satisfied and parrot existing ones, they are repeated to work around the lack of associated_type_bounds
- Optimized vector processing in the `fold` method of `src/provider/pedersen.rs`. Element-wise vector addition replaced the previous multiscalar multiplication method.
- Transitioned variable ck and ck_c to mutable to facilitate in-place operations.
- Optimized the `split_at` function in `Pedersen.rs` using the `split_off` method.
- Reconfigured the `combine` function to clone and chain iterables for expedited vector creation.
- Reassessed the `scale` function to in-place operations, eliminating the need for new struct instances.
- Streamlined code in `scale` and `fold` functions by removing superfluous variable assignments.
@huitseeker huitseeker added this pull request to the merge queue Feb 2, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to no response for status checks Feb 2, 2024
@huitseeker huitseeker added this pull request to the merge queue Feb 2, 2024
Merged via the queue into lurk-lab:dev with commit 6fde742 Feb 2, 2024
8 of 9 checks passed
@huitseeker huitseeker deleted the unmsm_ipa branch February 2, 2024 14:59
samuelburnham pushed a commit to samuelburnham/arecibo that referenced this pull request Feb 2, 2024
* add bitwise AND example (lurk-lab#289)

* add bitwise AND example

* fix cargo clippy

* Fix typos (lurk-lab#290)

* fix typo

* fix typo

* Fix typo

* Remove absorbing of running instance (lurk-lab#291)

* Remove absorbing of running instance

* Update value of NUM_FE_FOR_RO

* Update expected values in tests

* Add comment

* cargo fmt

* relax requirements on the size of public IO and add a note about NIFS (lurk-lab#294)

* relax requirements on the size of public IO

* add a note

---------

Co-authored-by: Srinath Setty <srinath@microsoft.com>
Co-authored-by: GoodDaisy <90915921+GoodDaisy@users.noreply.github.com>
Co-authored-by: Varun Thakore <vrnthakore@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants