Skip to content

chore(benchmark): Improve noir msm benchmark#7390

Merged
sirasistant merged 3 commits intomasterfrom
arv/improve_noir_msm_reference
Feb 19, 2025
Merged

chore(benchmark): Improve noir msm benchmark#7390
sirasistant merged 3 commits intomasterfrom
arv/improve_noir_msm_reference

Conversation

@sirasistant
Copy link
Contributor

Description

Problem*

Improves the MSM benchmark, by fixing some bugs and using double then add which minimizes work inside the loop.

Summary*

Additional Context

Documentation*

Check one:

  • No documentation needed.
  • Documentation included in this PR.
  • [For Experimental Features] Documentation to be submitted in a separate PR.

PR Checklist*

  • I have tested the changes locally.
  • I have formatted the changes with Prettier and/or cargo fmt on default settings.

@sirasistant sirasistant requested a review from vezenovm February 14, 2025 15:58
@github-actions
Copy link
Contributor

github-actions bot commented Feb 14, 2025

Changes to number of Brillig opcodes executed

Generated at commit: 49cc5b29f19736dfc00cf4bbcc7b36dec44093f4, compared to commit: 2b6db0749aa0f8d0065b913dc15f9a617bed258c

🧾 Summary (10% most significant diffs)

Program Brillig opcodes (+/-) %
multi_scalar_mul_inliner_min +33,456 ❌ +174.81%
multi_scalar_mul_inliner_max +16,540 ❌ +135.19%
multi_scalar_mul_inliner_zero +16,540 ❌ +135.19%

Full diff report 👇
Program Brillig opcodes (+/-) %
multi_scalar_mul_inliner_min 52,594 (+33,456) +174.81%
multi_scalar_mul_inliner_max 28,775 (+16,540) +135.19%
multi_scalar_mul_inliner_zero 28,775 (+16,540) +135.19%

@github-actions
Copy link
Contributor

github-actions bot commented Feb 14, 2025

Changes to Brillig bytecode sizes

Generated at commit: 49cc5b29f19736dfc00cf4bbcc7b36dec44093f4, compared to commit: 2b6db0749aa0f8d0065b913dc15f9a617bed258c

🧾 Summary (10% most significant diffs)

Program Brillig opcodes (+/-) %
multi_scalar_mul_inliner_min +29 ❌ +10.51%
multi_scalar_mul_inliner_max +16 ❌ +6.81%
multi_scalar_mul_inliner_zero +16 ❌ +6.81%

Full diff report 👇
Program Brillig opcodes (+/-) %
multi_scalar_mul_inliner_min 305 (+29) +10.51%
multi_scalar_mul_inliner_max 251 (+16) +6.81%
multi_scalar_mul_inliner_zero 251 (+16) +6.81%

@sirasistant
Copy link
Contributor Author

Let me update the scalars to some random fields...

@vezenovm
Copy link
Contributor

Let me update the scalars to some random fields...

You will want to submit as a separate PR so we can see the diff here. We could have a couple tests with different inputs.

@sirasistant
Copy link
Contributor Author

Is there a mechanism to have multiple Prover.tomls for benchmarking?

@sirasistant
Copy link
Contributor Author

If not I think it's better to have this benchmark with some scalars of different sizes

@vezenovm
Copy link
Contributor

Is there a mechanism to have multiple Prover.tomls for benchmarking?

Aside a workspace, no unfortunately. A workspace is the mechanism we use for generating the report from test_programs/execution_success.

@sirasistant sirasistant changed the title test: Improve noir msm benchmark chore(benchmark): Improve noir msm benchmark Feb 14, 2025
@vezenovm
Copy link
Contributor

If not I think it's better to have this benchmark with some scalars of different sizes

Yeah let's just update this benchmark for now.

@sirasistant
Copy link
Contributor Author

Now it has more realistic numbers for 5 scalars :D

@vezenovm vezenovm added this pull request to the merge queue Feb 18, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Feb 18, 2025
@sirasistant sirasistant added this pull request to the merge queue Feb 19, 2025
Merged via the queue into master with commit ba66d3b Feb 19, 2025
103 checks passed
@sirasistant sirasistant deleted the arv/improve_noir_msm_reference branch February 19, 2025 10:31
AztecBot pushed a commit to AztecProtocol/aztec-packages that referenced this pull request Feb 19, 2025
chore!: make `ResolverError::OracleMarkedAsConstrained` into a full error (noir-lang/noir#7426)
chore: simplify reports (noir-lang/noir#7421)
fix: do not discard negative sign from field literals in comptime interpreter (noir-lang/noir#7439)
chore: bump aztec-packages commit (noir-lang/noir#7441)
fix: require loop/for/while body to be unit (noir-lang/noir#7437)
feat: simplify assertions that squared values are equal to zero (noir-lang/noir#7432)
chore(benchmark): Improve noir msm benchmark (noir-lang/noir#7390)
chore: Add SSA security checks description (noir-lang/noir#7366)
TomAFrench added a commit to AztecProtocol/aztec-packages that referenced this pull request Feb 19, 2025
Automated pull of development from the
[noir](https://github.com/noir-lang/noir) programming language, a
dependency of Aztec.
BEGIN_COMMIT_OVERRIDE
feat(cli): add noir-execute binary
(noir-lang/noir#7384)
chore!: make `ResolverError::OracleMarkedAsConstrained` into a full
error (noir-lang/noir#7426)
chore: simplify reports (noir-lang/noir#7421)
fix: do not discard negative sign from field literals in comptime
interpreter (noir-lang/noir#7439)
chore: bump aztec-packages commit
(noir-lang/noir#7441)
fix: require loop/for/while body to be unit
(noir-lang/noir#7437)
feat: simplify assertions that squared values are equal to zero
(noir-lang/noir#7432)
chore(benchmark): Improve noir msm benchmark
(noir-lang/noir#7390)
chore: Add SSA security checks description
(noir-lang/noir#7366)
END_COMMIT_OVERRIDE

---------

Co-authored-by: Tom French <tom@tomfren.ch>
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