-
Notifications
You must be signed in to change notification settings - Fork 808
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
[keccak] Reduce Advice column usage in permutation circuit #501
Conversation
d6f4668
to
c65b055
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, Great improvement! Now we have much fewer columns overheads.
Added some nitpicks.
@@ -138,18 +136,9 @@ pub struct LaneRotateConversionConfig<F> { | |||
impl<F: Field> LaneRotateConversionConfig<F> { | |||
pub fn configure( | |||
meta: &mut ConstraintSystem<F>, | |||
lane_idx: usize, | |||
lane: Column<Advice>, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can still keep this lane and enable the copy for it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure I do follow. They'll be unused inside. So I thought the best was to enable it in the top level Config that instanciates LaneRotateConversionConfig
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Enabling it in the top-level Config sounds good.
We were previosly using 25 `BaseConversionConfig`s inside of a single `StateBaseConversion` config. Indeed, we can reach the same result and have 25 different regions for each BaseConversionConfig without the need to have 25 different instanciations. This is important since each one of them adds 6 Advice columns and 2 Selector columns. The after and before sum of columns for the permutation circuits shows a decent reduction of almost a half of the columns in usage.
Currently, we were generating 25 `LaneRotateConversionConfig`s inside of `RhoConfig`. The only way to prevent this behaviour passes by first refactoring `LaneRotateConversionConfig` so that it doesn't require `lane` and `lane_idx` in order to call `configure` and generate `rotation`. Therefore, the generation of `rotation` has been passed to the witness assignation step. And also, lane is now not required anymore. This allows to have 25 regions assigned for the config instead of 25 configs with 1 `Region` each.
As a consequence of f507d16 and continuing the work on #489 this reduces the `RhoConfig` to use a simple `LaneRotateConversionConfig` which gets assigned 25 times witnesses generating 25 different `Region`s instead of duplicating columns over and over. This is important since each one of them adds 5 `Advice` columns, 2 `Fixed` columns and 2 Selector columns.
c65b055
to
f8b7764
Compare
Final benchmarks before/after for keccak permutation after the column reduction/squash.
After:
|
Advice columns collapsing
StateBaseConversion - BaseConversion
We were previosly using 25
BaseConversionConfig
s inside of a singleStateBaseConversion
config.Indeed, we can reach the same result and have 25 different regions for
each BaseConversionConfig without the need to have 25 different
instanciations.
This is important since each one of them adds 6 Advice columns and 2
Selector columns.
The after and before sum of columns for the permutation circuits shows a
decent reduction of almost a half of the columns in usage.
Before results for
test_state_base_conversion
:After results for
test_state_base_conversion
:Rho - LaneRotateConversionConfig
As a consequence of f507d16 and continuing the work on #489 this reduces
the
RhoConfig
to use a simpleLaneRotateConversionConfig
which getsassigned 25 times witnesses generating 25 different
Region
s instead ofduplicating columns over and over.
This is important since each one of them adds 5
Advice
columns, 2Fixed
columns and 2 Selector columns.Before results for
test_rho_gate
:After results for
test_rho_gate
: