Skip to content

chore: deduplicate batch affine addition trick#19788

Merged
iakovenkos merged 53 commits intomerge-train/barretenbergfrom
si/unify-batch-affine-add
Jan 23, 2026
Merged

chore: deduplicate batch affine addition trick#19788
iakovenkos merged 53 commits intomerge-train/barretenbergfrom
si/unify-batch-affine-add

Conversation

@iakovenkos
Copy link
Contributor

@iakovenkos iakovenkos commented Jan 21, 2026

Montgomery batch inversion trick was implemented in 3 different places, now all of them are using generic impl that has a policy abstraction allowing to support different memory requirements (e.g. pippenger vs scaling a vector of commitments by the same field element)

Closes AztecProtocol/barretenberg#826

iakovenkos and others added 30 commits December 23, 2025 12:34
- Rename get_nonzero_scalar_indices to transform_scalar_and_get_nonzero_scalar_indices
- Perform lazy Montgomery conversion during zero filtering (once per scalar)
- Remove redundant conversion loop in msm() function
- Restores baseline performance (743.149G vs 742.837G cycles, 0.04% difference)
- Eliminates 6-9% regression from eager conversion approach

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
- Revert from recursive back to iterative implementation
- Iterative version is cleaner and more readable with explicit loops
- Fix addition_result_bucket_destinations type from uint64_t to uint32_t
- Eliminates unnecessary type casts throughout
- Keep BB_BENCH profiling markers for performance tracking
- Performance remains identical to recursive version

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
@iakovenkos iakovenkos marked this pull request as ready for review January 21, 2026 12:40
````

When making barretenberg changes, ensure these tests still pass.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Claude would often try to create a custom benching script, or bench perf critical primitives locally.

Copy link
Contributor

@federicobarbacovi federicobarbacovi left a comment

Choose a reason for hiding this comment

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

Great stuff!

using AffineElement = typename Curve::AffineElement;
using BaseField = typename Curve::BaseField;

// Use interleaved array policy: pairs are (points[2i], points[2i+1]), output in points[num_pairs:]
Copy link
Contributor

Choose a reason for hiding this comment

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

Refuse: output in points[num_pairs + i]

Base automatically changed from si/pippenger-audit-0 to merge-train/barretenberg January 23, 2026 15:37
@iakovenkos iakovenkos merged commit 8332909 into merge-train/barretenberg Jan 23, 2026
8 checks passed
@iakovenkos iakovenkos deleted the si/unify-batch-affine-add branch January 23, 2026 17:00
github-merge-queue bot pushed a commit that referenced this pull request Jan 27, 2026
BEGIN_COMMIT_OVERRIDE
feat: support JSON input files for bb verify command (#19800)
fix: update bootstrap.sh to use new JSON field names
chore: Update `index.js` so that `HAS_ZK` and `PUBLIC_INPUTS` variables
must always be set in tests (#19884)
chore: pippenger int audit (#19302)
chore: deduplicate batch affine addition trick (#19788)
chore: transcript+codec+poseidon2 fixes (#19419)
chore!: explicitly constrain inputs and intermediate witnesses (#19826)
fix: exclude nlohmann/json from WASM builds in json_output.hpp
chore: translator circuit builder and flavor audit (#19798)
Revert "fix: exclude nlohmann/json from WASM builds in json_output.hpp"
Revert "feat: support JSON input files for bb verify command (#19800)"
Revert "fix: update bootstrap.sh to use new JSON field names"
END_COMMIT_OVERRIDE
danielntmd pushed a commit that referenced this pull request Jan 27, 2026
Montgomery batch inversion trick was implemented in 3 different places,
now all of them are using generic impl that has a policy abstraction
allowing to support different memory requirements (e.g. pippenger vs
scaling a vector of commitments by the same field element)

Closes AztecProtocol/barretenberg#826

---------

Co-authored-by: Claude Sonnet 4.5 <noreply@anthropic.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.

2 participants