Witness multiplication optimization #70
Conversation
| @@ -356,14 +360,16 @@ impl<F: PrimeField> AHPForR1CS<F> { | |||
|
|
|||
| let z_a_poly_time = start_timer!(|| "Computing z_A polynomial"); | |||
| let z_a = state.z_a.clone().unwrap(); | |||
| let z_a_poly = &EvaluationsOnDomain::from_vec_and_domain(z_a, domain_h).interpolate() | |||
| + &(&DensePolynomial::from_coefficients_slice(&[F::rand(rng)]) * &v_H); | |||
| let r_a = F::rand(rng); | |||
There was a problem hiding this comment.
Can there be a comment here explaining the operation these 3 lines are doing? (Ditto for z_b)
I believe its "go from z_a evaluations to z_a_poly as interpolate(z_a_evaluations over H) * (Z_I)) + rand_poly
There was a problem hiding this comment.
It's computed as interpolate(z_a_evaluations_over_H) + v_H * r_a
src/ahp/prover.rs
Outdated
| mz_rands: None, | ||
| mz_polys: None, | ||
| mz_rand_polys: None, |
There was a problem hiding this comment.
I don't think we need to store both mz_polys and mz_rand_polys; we can store just mz_rand_polys, and then later on, when we need to compute z_a * z_b, we just temporarily mutate mz_rand_polys in place by subtracting mz_rands * v_H.
This should reduce memory consumption.
| let v_H: DensePolynomial<F> = domain_h.vanishing_polynomial().into(); | ||
| let z_a_poly = randomized_z_a_poly.polynomial() - &(&v_H * r_a); |
There was a problem hiding this comment.
Oh hm I meant that we should instead mutate randomized_z_a_poly in place temporarily. Also, we shouldn't convert v_H into a dense polynomial, as that wastes memory. Instead, we should add a way to subtract a sparse polynomial from a dense polynomial (if that doesn't already exist)
There was a problem hiding this comment.
(and also a way to multiply a sparse polynomial by a scalar, mirroring the one for DensePolynomial
There was a problem hiding this comment.
Gotcha, so the problem with that is randomized_z_a_poly and randomized_z_b_poly are inside an Rc that is borrowed by ProverFirstOracles here until the end of the protocol. So we could mutate in place, but I believe this would require unsafe to do (which I'm fine doing because we know the other reference isn't used until later) - or am I missing something?
I'll make the other changes mentioned for now though!
Description
Currently, the witness polynomial multiplication in the second round of the AHP requires performing an FFT with a domain of size 4 * |H|. This change distributes the multiplication so that it only requires an FFT with a domain of size 2 * |H| along with some cheap multiplications/additions.
Running marlin-benches on a fairly powerful machine from
master:Running marlin-benches on the same machine from this PR:
So it seems to give an overall slight latency improvement and reduces the memory overhead of proving. I would guess that performance gains would be more noticeable on weaker machines.
This PR depends on arkworks-rs/algebra#258
Before we can merge this PR, please make sure that all the following items have been
checked off. If any of the checklist items are not applicable, please leave them but
write a little note why.
Pendingsection inCHANGELOG.mdFiles changedin the Github PR explorer