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

[Release v0.4.0] FFT memory reduction and proving key size reduction #17

Merged
merged 8 commits into from
Nov 18, 2023

Conversation

jonathanpwang
Copy link
Collaborator

@jonathanpwang jonathanpwang commented Oct 27, 2023

Cherry-picked from taikoxyz#6 however the original PR for this is scroll-tech#28

In the process of debugging I also cherry-picked taikoxyz#4 as cleanup.

The proving key size is reduced because the extended Lagrange coefficients of fixed polynomials is not stored in the proving key. However this means this FFT computation is done by the prover. We need to benchmark further to determine whether we want this prover speed tradeoff for the decrease in proving key size and decreased memory usage.

I have also removed the shuffle API because it is not supported by snark-verifier.

Preliminary benchmark on my laptop:
Using https://github.com/axiom-crypto/halo2-lib/tree/0df530e6e29a591a6c7030953a140163e8991bd3 for keccak shard + aggregation.

On main branch without this PR:

# shard_k = 15, agg_k = 18, keccak rows_per_round = 21
cargo t single_layer_aggregate_leaf_circuits_commit::_21_rows_per_round -- --nocapture
# shard:
End:     Create proof ..............................................................16.175s
# aggregation:
End:     Create proof ..............................................................79.420s

Using this PR:

cargo t single_layer_aggregate_leaf_circuits_commit::_21_rows_per_round -- --nocapture
# shard:
End:     Create proof ..............................................................16.476s
# aggregation:
End:     Create proof ..............................................................98.437s

Aggregation circuit stats:

BaseCircuitParams {
        k: 18,
        num_advice_per_phase: [
            134,
        ],
        num_fixed: 1,
        num_lookup_advice_per_phase: [
            16,
            0,
            0,
        ],
        lookup_bits: Some(
            17,
        ),
        num_instance_columns: 1,
    }

We have decided that the reduction in memory bandwidth and proving key size is worth the performance overhead, so this will be merged as halo2-axiom release v0.4.0.

@jonathanpwang jonathanpwang changed the title [HOLD] [feat] FFT memory reduction and proving key size reduction [feat] FFT memory reduction and proving key size reduction Nov 18, 2023
@jonathanpwang jonathanpwang changed the title [feat] FFT memory reduction and proving key size reduction [Release v0.4.0] FFT memory reduction and proving key size reduction Nov 18, 2023
@jonathanpwang jonathanpwang enabled auto-merge (squash) November 18, 2023 00:18
@jonathanpwang jonathanpwang merged commit e841084 into main Nov 18, 2023
8 checks passed
@jonathanpwang jonathanpwang deleted the feat/fft-mem-optimization branch November 18, 2023 00:19
CPerezz added a commit to privacy-scaling-explorations/halo2 that referenced this pull request Feb 7, 2024
This incorporates the work done in
scroll-tech#28 in order to lower the
memory consumption significantly trading off for some performance.

A much more deep analysis can be found here: axiom-crypto#17
CPerezz added a commit to privacy-scaling-explorations/halo2 that referenced this pull request Feb 26, 2024
This incorporates the work done in
scroll-tech#28 in order to lower the
memory consumption significantly trading off for some performance.

A much more deep analysis can be found here: axiom-crypto#17
CPerezz added a commit to privacy-scaling-explorations/halo2 that referenced this pull request Mar 2, 2024
This incorporates the work done in
scroll-tech#28 in order to lower the
memory consumption significantly trading off for some performance.

A much more deep analysis can be found here: axiom-crypto#17
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.

2 participants