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

The Return of the King #530

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Open

Conversation

arasuarun
Copy link
Collaborator

Optimization crushing the second Spartan sumcheck. Instead of the inner sumcheck with a * b rounds where a = num_vars_padded and b = num_steps_padded, we now perform two sumchecks of a rounds and b rounds, respectively.

There's a lot to document. I'm making the PR for now to unblock @moodlezoup's other optimizations.

A major change in the code that we should discuss before merging:

  • There was an issue with how the cross-step (also called "non-uniform" constraints) were handled for the last step. Now, the changes in this PR assume that all witness elements belonging to the last step are 0, so the step is effectively ignored. This requires some changes to the trace polynomials (in builder.rs).
  • In particular, this means that if a program runs for exactly a power of 2 number of steps, we'd still need to pad it to the next power of 2. I don't think this is an issue for now but might pop up if we're sharding programs later for folding.

How ready is the PR? It's good enough to build on but needs cleanup before merging into main.

  • Needs code cleanup like variable names and so on (especially in key.rs).
  • I don't think there are any major wasteful clones.
  • Needs a sweep to see which parts can be parallelized better. Particularly a loop in the prover function that possibly runs into some of the old paging issues we faced before.

Also need to test with flamegraphs to make sure we're seeing the right performance improvements in the prover's sumchecks.

@arasuarun arasuarun requested a review from moodlezoup December 16, 2024 22:52
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.

1 participant