search

privacy-scaling-explorations / halo2

https://privacy-scaling-explorations.github.io/halo2/
Other
202 stars 122 forks source link

Investigate improvements for assign_region #155

Open pinkiebell opened 1 year ago

pinkiebell commented 1 year ago

At this moment, the SimpleFloorPlanner used in zkevm-circuits will call the closure for assign_region twice. It might makes sense to deduplicate this call in halo2 because there a few compute heavy loops inside zkevm-circuits that not only does assignments but also data mapping / transformation and right now, doing the work twice. We can gain significant speedups by either i) modifying zkevm-circuits to do the heavy work outside the closure and reference it instead ii) find a way to avoid calling more than 1 times in halo2 - this generally improves performance for any circuit/crate

pinkiebell commented 1 year ago

ref: https://github.com/privacy-scaling-explorations/zkevm-circuits/pull/1253#issuecomment-1447785851 cc/ @CPerezz

jonathanpwang commented 1 year ago

I have a PR for this here: https://github.com/halo2-ce/halo2/pull/13 You can cherry-pick the last 2 commits and test it out.

pinkiebell commented 1 year ago

I have a PR for this here: halo2-ce#13 You can cherry-pick the last 2 commits and test it out.

I've tested that and while it makes the synthesize step faster (2x) it greatly increases the degree: https://github.com/privacy-scaling-explorations/zkevm-chain/pull/133/commits/f16e43da08dcf4c71ca915c4fd1e79c55956ce09 .

Maybe we can store all information from assignments and then do the shaping instead calling it multiple times?

CPerezz commented 1 year ago

I've tested that and while it makes the synthesize step faster (2x) it greatly increases the degree: https://github.com/privacy-scaling-explorations/zkevm-chain/commit/f16e43da08dcf4c71ca915c4fd1e79c55956ce09 .

I don;t think that's an issue. While we keep the expr degree low (that's of course the case as it isn't touched) sacrifice 1 degree in the MockProver is not an issue at all IMO (Specially considering that this Layouter should never be used for final circuits/benchmarks).

Which is your proposal @pinkiebell ? I thought it's not a big deal to get a bigger degree (specially since we don't need a setup with the MockProver.

pinkiebell commented 1 year ago

It would be a great performance improvement for real & mock prover alike to improve halo2 to not calling these closures from assign_region etc. twice. Because changing all the circuits to take this into account is, I think, a lot of unnecessary work if that can be fixed in the halo2 crate. We have to keep in mind that for production circuits with a large fixed paddings the performance penalty goes into dozens of minutes like > 30mins. I can take a look at the halo2 code because to assess this task I don't have enough information about it yet.