Skip to content

fix: civc wasm memory regression#15722

Merged
iakovenkos merged 6 commits into
merge-train/barretenbergfrom
si/mem-reg-bench
Jul 15, 2025
Merged

fix: civc wasm memory regression#15722
iakovenkos merged 6 commits into
merge-train/barretenbergfrom
si/mem-reg-bench

Conversation

@iakovenkos

@iakovenkos iakovenkos commented Jul 15, 2025

Copy link
Copy Markdown
Contributor

In my previous PR, I moved the GateSeparatorPolynomial into the SumcheckProver class as a member, which extended its lifetime and increased memory usage in CIVC by the size of this polynomial, e.g. by 21 MB in ecdsar1+transfer_0_recursions+sponsored_fpc bench

@iakovenkos

iakovenkos commented Jul 15, 2025

Copy link
Copy Markdown
Contributor Author
image as opposed to 831 in `next` benchmarks

@iakovenkos iakovenkos changed the base branch from next to merge-train/barretenberg July 15, 2025 09:59
// Zero univariates are used to pad the proof to the fixed size virtual_log_n.
auto zero_univariate = bb::Univariate<FF, Flavor::BATCHED_RELATION_PARTIAL_LENGTH>::zero();
for (size_t idx = multivariate_d; idx < virtual_log_n; idx++) {
if constexpr (!IsGrumpkinFlavor<Flavor>) {

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This loop is never triggered when IsGrumkinFlavor<Flavor> == true

@johnathan79717 johnathan79717 Jul 15, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe it's worth adding something like an ASSERT(!IsGrumpkinFlavor<Flavor>); to make sure.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

note you can do static_assert() also

SubrelationSeparators alphas;
// pow_β(X₀, ..., X_{d−1}) = ∏ₖ₌₀^{d−1} (1 − Xₖ + Xₖ ⋅ βₖ)
bb::GateSeparatorPolynomial<FF> gate_separators;
std::vector<FF> gate_challenges;

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the size of gate_challenges is ~ log of the circuit size

// Given gate challenges β = (β₀, ..., β_{d−1}) and d = `multivariate_d`, compute the evaluations of
// GateSeparator_β (X₀, ..., X_{d−1}) = ∏ₖ₌₀^{d−1} (1 − Xₖ + Xₖ · βₖ)
// on the boolean hypercube.
GateSeparatorPolynomial<FF> gate_separators(gate_challenges, multivariate_d);

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

GateSeparator is a poly of circuit size, so should be created locally

@iakovenkos iakovenkos self-assigned this Jul 15, 2025
@iakovenkos iakovenkos marked this pull request as ready for review July 15, 2025 10:15

@johnathan79717 johnathan79717 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

// Zero univariates are used to pad the proof to the fixed size virtual_log_n.
auto zero_univariate = bb::Univariate<FF, Flavor::BATCHED_RELATION_PARTIAL_LENGTH>::zero();
for (size_t idx = multivariate_d; idx < virtual_log_n; idx++) {
if constexpr (!IsGrumpkinFlavor<Flavor>) {

@johnathan79717 johnathan79717 Jul 15, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe it's worth adding something like an ASSERT(!IsGrumpkinFlavor<Flavor>); to make sure.

@iakovenkos iakovenkos merged commit 3eb8414 into merge-train/barretenberg Jul 15, 2025
9 checks passed
@iakovenkos iakovenkos deleted the si/mem-reg-bench branch July 15, 2025 11:13
github-merge-queue Bot pushed a commit that referenced this pull request Jul 16, 2025
See
[merge-train-readme.md](https://github.com/AztecProtocol/aztec-packages/blob/next/.github/workflows/merge-train-readme.md).

BEGIN_COMMIT_OVERRIDE
feat!: Merge with degree check (#15562)
fix: Fix the docker build action for fuzzing (#15719)
fix: restore accidentally deleted files (#15724)
fix: civc wasm memory regression (#15722)
feat: mmap backed polynomials (#15531)
feat(bbapi): CLI uses bbapi CIVC (#15702)
fix(ci): brittle benchmark behavior (#15771)
END_COMMIT_OVERRIDE

---------

Co-authored-by: AztecBot <tech@aztecprotocol.com>
Co-authored-by: federicobarbacovi <171914500+federicobarbacovi@users.noreply.github.com>
Co-authored-by: sergei iakovenko <105737703+iakovenkos@users.noreply.github.com>
Co-authored-by: ludamad <adam.domurad@gmail.com>
Co-authored-by: Suyash Bagad <suyash@aztecprotocol.com>
Co-authored-by: Jonathan Hao <jonathan@aztec-labs.com>
Co-authored-by: ledwards2225 <98505400+ledwards2225@users.noreply.github.com>
Co-authored-by: Raju Krishnamoorthy <krishnamoorthy@gmail.com>
Co-authored-by: notnotraju <raju@aztec-labs.com>
Co-authored-by: Lucas Xia <lucasxia01@gmail.com>
Co-authored-by: Khashayar Barooti <khashayar@aztecprotocol.com>
Co-authored-by: Jean M <132435771+jeanmon@users.noreply.github.com>
Co-authored-by: Alex Gherghisan <alexghr@users.noreply.github.com>
Co-authored-by: Santiago Palladino <spalladino@users.noreply.github.com>
Co-authored-by: Santiago Palladino <santiago@aztec-labs.com>
Co-authored-by: ludamad <domuradical@gmail.com>
Co-authored-by: maramihali <mara@aztecprotocol.com>
Co-authored-by: Sarkoxed <75146596+Sarkoxed@users.noreply.github.com>
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.

4 participants