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

feat: avoid generating a new witness when checking if linear expression is zero #7031

Merged
merged 4 commits into from
Jan 13, 2025

Conversation

TomAFrench
Copy link
Member

…sion is zero

Description

Problem*

Resolves

Summary*

We're currently creating a new witness when checking whether linear expressions are equal to zero. This is unnecessary as these will neatly fit into the following constraints (at least for direct scaling of a single witness)

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.

@TomAFrench TomAFrench changed the title feat: avoid generating a new witness when checking when linear expres… feat: avoid generating a new witness when checking if linear expression is zero Jan 11, 2025
Copy link
Contributor

github-actions bot commented Jan 11, 2025

Changes to circuit sizes

Generated at commit: 1058cd1d47bb4bee7acb578206c17b4c02597274, compared to commit: db28cb9ffb710c286b54dbfcf57292ae3dffb03d

🧾 Summary (10% most significant diffs)

Program ACIR opcodes (+/-) % Circuit size (+/-) %
conditional_1 -141 ✅ -4.70% -141 ✅ -1.35%
references -1 ✅ -12.50% -1 ✅ -4.35%
type_aliases -1 ✅ -16.67% -1 ✅ -4.76%
brillig_conditional -1 ✅ -16.67% -1 ✅ -5.00%
negated_jmpif_condition -1 ✅ -20.00% -1 ✅ -5.00%
debug_logs -1 ✅ -2.50% -1 ✅ -5.26%

Full diff report 👇
Program ACIR opcodes (+/-) % Circuit size (+/-) %
bench_eddsa_poseidon 16,438 (-1) -0.01% 19,585 (-1) -0.01%
keccak256 123 (-2) -1.60% 20,145 (-2) -0.01%
nested_array_dynamic 3,351 (-2) -0.06% 12,994 (-2) -0.02%
embedded_curve_ops 22 (-1) -4.35% 5,849 (-1) -0.02%
nested_array_in_slice 958 (-1) -0.10% 5,554 (-1) -0.02%
hashmap 36,900 (-22) -0.06% 98,580 (-22) -0.02%
binary_operator_overloading 122 (-1) -0.81% 4,262 (-1) -0.02%
sha256_regression 22,342 (-44) -0.20% 180,386 (-44) -0.02%
regression_capacity_tracker 115 (-1) -0.86% 3,970 (-1) -0.03%
regression 292 (-1) -0.34% 3,827 (-1) -0.03%
conditional_regression_short_circuit 367 (-3) -0.81% 11,139 (-3) -0.03%
regression_mem_op_predicate 44 (-1) -2.22% 3,567 (-1) -0.03%
5_over 20 (-1) -4.76% 3,488 (-1) -0.03%
4_sub 13 (-1) -7.14% 3,477 (-1) -0.03%
xor 7 (-1) -12.50% 3,146 (-1) -0.03%
global_var_regression_simple 130 (-1) -0.76% 3,068 (-1) -0.03%
regression_4709 69 (-1) -1.43% 2,916 (-1) -0.03%
u16_support 69 (-1) -1.43% 2,892 (-1) -0.03%
arithmetic_binary_operations 15 (-1) -6.25% 2,787 (-1) -0.04%
array_dynamic 100 (-2) -1.96% 3,718 (-2) -0.05%
tuple_inputs 41 (-2) -4.65% 3,647 (-2) -0.05%
simple_comparison 22 (-2) -8.33% 3,494 (-2) -0.06%
brillig_not 22 (-2) -8.33% 2,800 (-2) -0.07%
conditional_2 17 (-2) -10.53% 2,776 (-2) -0.07%
bit_shifts_runtime 284 (-3) -1.05% 3,945 (-3) -0.08%
signed_division 200 (-3) -1.48% 3,675 (-3) -0.08%
sha256 1,436 (-24) -1.64% 24,583 (-24) -0.10%
wrapping_operations 31 (-3) -8.82% 2,942 (-3) -0.10%
regression_5252 32,862 (-54) -0.16% 44,465 (-54) -0.12%
bit_and 42 (-4) -8.70% 3,058 (-4) -0.13%
sha256_var_witness_const_regression 1,075 (-23) -2.09% 16,477 (-23) -0.14%
slice_dynamic_index 954 (-9) -0.93% 6,345 (-9) -0.14%
sha256_var_size_regression 10,192 (-93) -0.90% 62,963 (-93) -0.15%
bigint 1,031 (-31) -2.92% 8,413 (-31) -0.37%
u128 466 (-20) -4.12% 4,425 (-20) -0.45%
6_array 392 (-21) -5.08% 4,031 (-21) -0.52%
hash_to_field 454 (-30) -6.20% 3,489 (-30) -0.85%
sha2_byte 15,229 (-711) -4.46% 81,964 (-711) -0.86%
slices 678 (-37) -5.17% 3,779 (-37) -0.97%
ram_blowup_regression 140,756 (-7,680) -5.17% 643,259 (-7,680) -1.18%
conditional_1 2,860 (-141) -4.70% 10,289 (-141) -1.35%
references 7 (-1) -12.50% 22 (-1) -4.35%
type_aliases 5 (-1) -16.67% 20 (-1) -4.76%
brillig_conditional 5 (-1) -16.67% 19 (-1) -5.00%
negated_jmpif_condition 4 (-1) -20.00% 19 (-1) -5.00%
debug_logs 39 (-1) -2.50% 18 (-1) -5.26%

Copy link
Contributor

github-actions bot commented Jan 11, 2025

Execution Memory Report

Program Peak Memory %
keccak256 74.67M 0%
workspace 123.79M 0%
regression_4709 316.00M 0%
ram_blowup_regression 512.61M 0%
rollup-base-public 478.82M -1%
rollup-base-private 325.41M 0%
private-kernel-tail 180.42M 0%
private-kernel-reset 245.11M 0%
private-kernel-inner 208.50M 0%

Copy link
Contributor

github-actions bot commented Jan 11, 2025

Compilation Memory Report

Program Peak Memory
keccak256 78.13M
workspace 123.77M
regression_4709 424.09M
ram_blowup_regression 1.58G
rollup-base-public 4.85G
rollup-base-private 1.26G
private-kernel-tail 207.13M
private-kernel-reset 669.21M
private-kernel-inner 294.35M

Copy link
Contributor

github-actions bot commented Jan 11, 2025

Execution Report

Program Execution Time %
sha256_regression 0.052s -2%
regression_4709 0.001s 0%
ram_blowup_regression 0.602s -6%
rollup-root 0.105s 0%
rollup-block-merge 0.105s -1%
rollup-base-public 1.224s -2%
rollup-base-private 0.450s -2%
private-kernel-tail 0.019s 0%
private-kernel-reset 0.312s -1%
private-kernel-inner 0.069s -5%

Copy link
Contributor

github-actions bot commented Jan 11, 2025

Compilation Report

Program Compilation Time %
sha256_regression 1.268s 0%
regression_4709 0.800s -2%
ram_blowup_regression 17.900s -1%
rollup-root 3.528s 5%
rollup-block-merge 3.392s 0%
rollup-base-public 41.200s -3%
rollup-base-private 13.180s 1%
private-kernel-tail 1.020s -5%
private-kernel-reset 6.896s -5%
private-kernel-inner 2.202s 0%

@TomAFrench TomAFrench requested a review from a team January 13, 2025 09:34
@TomAFrench TomAFrench enabled auto-merge January 13, 2025 10:06
Copy link
Contributor

@aakoshh aakoshh left a comment

Choose a reason for hiding this comment

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

Nice one, great reduction in bytecode size 👏

As I mentioned it would be nice to make the y_is_boolean_constraint expression more fluid, but otherwise looks correct 👍

@TomAFrench TomAFrench added this pull request to the merge queue Jan 13, 2025
Merged via the queue into master with commit 14c9237 Jan 13, 2025
87 checks passed
@TomAFrench TomAFrench deleted the tf/improve-equality-acirgen branch January 13, 2025 11:28
AztecBot added a commit to AztecProtocol/aztec-packages that referenced this pull request Jan 13, 2025
…oir-lang/noir#7037)

feat: avoid inserting `inc_rc` instructions into ACIR (noir-lang/noir#7036)
fix(lsp): suggest all possible trait methods, but only visible ones (noir-lang/noir#7027)
chore: add more protocol circuits to reports (noir-lang/noir#6977)
feat: avoid generating a new witness when checking if linear expression is zero (noir-lang/noir#7031)
feat: skip codegen of zero iteration loops (noir-lang/noir#7030)
AztecBot added a commit to AztecProtocol/aztec-packages that referenced this pull request Jan 14, 2025
…ake averages with single runs (noir-lang/noir#7048)

fix(nargo_fmt): let doc comment could come after regular comment (noir-lang/noir#7046)
fix(nargo_fmt): don't consider identifiers the same if they are equal… (noir-lang/noir#7043)
feat: auto-import traits when suggesting trait methods (noir-lang/noir#7037)
feat: avoid inserting `inc_rc` instructions into ACIR (noir-lang/noir#7036)
fix(lsp): suggest all possible trait methods, but only visible ones (noir-lang/noir#7027)
chore: add more protocol circuits to reports (noir-lang/noir#6977)
feat: avoid generating a new witness when checking if linear expression is zero (noir-lang/noir#7031)
feat: skip codegen of zero iteration loops (noir-lang/noir#7030)
AztecBot added a commit to AztecProtocol/aztec-packages that referenced this pull request Jan 14, 2025
… with single runs (noir-lang/noir#7048)

fix(nargo_fmt): let doc comment could come after regular comment (noir-lang/noir#7046)
fix(nargo_fmt): don't consider identifiers the same if they are equal… (noir-lang/noir#7043)
feat: auto-import traits when suggesting trait methods (noir-lang/noir#7037)
feat: avoid inserting `inc_rc` instructions into ACIR (noir-lang/noir#7036)
fix(lsp): suggest all possible trait methods, but only visible ones (noir-lang/noir#7027)
chore: add more protocol circuits to reports (noir-lang/noir#6977)
feat: avoid generating a new witness when checking if linear expression is zero (noir-lang/noir#7031)
feat: skip codegen of zero iteration loops (noir-lang/noir#7030)
AztecBot added a commit to AztecProtocol/aztec-packages that referenced this pull request Jan 14, 2025
…oir-lang/noir#7066)

feat!: Handle generic fields in `StructDefinition::fields` and move old functionality to `StructDefinition::fields_as_written` (noir-lang/noir#7067)
chore(ci): Check various inliner aggressiveness setttings in Brillig reports (noir-lang/noir#7049)
chore: reenable reports on rollup root circuits (noir-lang/noir#7061)
fix: don't always select trait impl when verifying trait constraints (noir-lang/noir#7041)
chore: mark some critical libraries as good again (noir-lang/noir#7065)
fix: Reduce memory usage in mem2reg (noir-lang/noir#7053)
feat: Allow associated types to be ellided from trait constraints (noir-lang/noir#7026)
chore(perf): try using vec-collections's VecSet in AliasSet (noir-lang/noir#7058)
chore: reduce number of iterations of `acvm::compiler::compile` (noir-lang/noir#7050)
chore: add `noir_check_shuffle` as a critical library (noir-lang/noir#7056)
chore: clippy warning fix (noir-lang/noir#7051)
chore(ci): Unify compilation/execution report jobs that take averages with single runs  (noir-lang/noir#7048)
fix(nargo_fmt): let doc comment could come after regular comment (noir-lang/noir#7046)
fix(nargo_fmt): don't consider identifiers the same if they are equal… (noir-lang/noir#7043)
feat: auto-import traits when suggesting trait methods (noir-lang/noir#7037)
feat: avoid inserting `inc_rc` instructions into ACIR (noir-lang/noir#7036)
fix(lsp): suggest all possible trait methods, but only visible ones (noir-lang/noir#7027)
chore: add more protocol circuits to reports (noir-lang/noir#6977)
feat: avoid generating a new witness when checking if linear expression is zero (noir-lang/noir#7031)
feat: skip codegen of zero iteration loops (noir-lang/noir#7030)
AztecBot added a commit to AztecProtocol/aztec-packages that referenced this pull request Jan 14, 2025
…ir#7066)

feat!: Handle generic fields in `StructDefinition::fields` and move old functionality to `StructDefinition::fields_as_written` (noir-lang/noir#7067)
chore(ci): Check various inliner aggressiveness setttings in Brillig reports (noir-lang/noir#7049)
chore: reenable reports on rollup root circuits (noir-lang/noir#7061)
fix: don't always select trait impl when verifying trait constraints (noir-lang/noir#7041)
chore: mark some critical libraries as good again (noir-lang/noir#7065)
fix: Reduce memory usage in mem2reg (noir-lang/noir#7053)
feat: Allow associated types to be ellided from trait constraints (noir-lang/noir#7026)
chore(perf): try using vec-collections's VecSet in AliasSet (noir-lang/noir#7058)
chore: reduce number of iterations of `acvm::compiler::compile` (noir-lang/noir#7050)
chore: add `noir_check_shuffle` as a critical library (noir-lang/noir#7056)
chore: clippy warning fix (noir-lang/noir#7051)
chore(ci): Unify compilation/execution report jobs that take averages with single runs  (noir-lang/noir#7048)
fix(nargo_fmt): let doc comment could come after regular comment (noir-lang/noir#7046)
fix(nargo_fmt): don't consider identifiers the same if they are equal… (noir-lang/noir#7043)
feat: auto-import traits when suggesting trait methods (noir-lang/noir#7037)
feat: avoid inserting `inc_rc` instructions into ACIR (noir-lang/noir#7036)
fix(lsp): suggest all possible trait methods, but only visible ones (noir-lang/noir#7027)
chore: add more protocol circuits to reports (noir-lang/noir#6977)
feat: avoid generating a new witness when checking if linear expression is zero (noir-lang/noir#7031)
feat: skip codegen of zero iteration loops (noir-lang/noir#7030)
AztecBot added a commit to AztecProtocol/aztec-packages that referenced this pull request Jan 15, 2025
…ir#7070)

feat(LSP): code action to import trait in a method call (noir-lang/noir#7066)
feat!: Handle generic fields in `StructDefinition::fields` and move old functionality to `StructDefinition::fields_as_written` (noir-lang/noir#7067)
chore(ci): Check various inliner aggressiveness setttings in Brillig reports (noir-lang/noir#7049)
chore: reenable reports on rollup root circuits (noir-lang/noir#7061)
fix: don't always select trait impl when verifying trait constraints (noir-lang/noir#7041)
chore: mark some critical libraries as good again (noir-lang/noir#7065)
fix: Reduce memory usage in mem2reg (noir-lang/noir#7053)
feat: Allow associated types to be ellided from trait constraints (noir-lang/noir#7026)
chore(perf): try using vec-collections's VecSet in AliasSet (noir-lang/noir#7058)
chore: reduce number of iterations of `acvm::compiler::compile` (noir-lang/noir#7050)
chore: add `noir_check_shuffle` as a critical library (noir-lang/noir#7056)
chore: clippy warning fix (noir-lang/noir#7051)
chore(ci): Unify compilation/execution report jobs that take averages with single runs  (noir-lang/noir#7048)
fix(nargo_fmt): let doc comment could come after regular comment (noir-lang/noir#7046)
fix(nargo_fmt): don't consider identifiers the same if they are equal… (noir-lang/noir#7043)
feat: auto-import traits when suggesting trait methods (noir-lang/noir#7037)
feat: avoid inserting `inc_rc` instructions into ACIR (noir-lang/noir#7036)
fix(lsp): suggest all possible trait methods, but only visible ones (noir-lang/noir#7027)
chore: add more protocol circuits to reports (noir-lang/noir#6977)
feat: avoid generating a new witness when checking if linear expression is zero (noir-lang/noir#7031)
feat: skip codegen of zero iteration loops (noir-lang/noir#7030)
AztecBot added a commit to AztecProtocol/aztec-packages that referenced this pull request Jan 15, 2025
feat(LSP): code action to import trait in a method call (noir-lang/noir#7066)
feat!: Handle generic fields in `StructDefinition::fields` and move old functionality to `StructDefinition::fields_as_written` (noir-lang/noir#7067)
chore(ci): Check various inliner aggressiveness setttings in Brillig reports (noir-lang/noir#7049)
chore: reenable reports on rollup root circuits (noir-lang/noir#7061)
fix: don't always select trait impl when verifying trait constraints (noir-lang/noir#7041)
chore: mark some critical libraries as good again (noir-lang/noir#7065)
fix: Reduce memory usage in mem2reg (noir-lang/noir#7053)
feat: Allow associated types to be ellided from trait constraints (noir-lang/noir#7026)
chore(perf): try using vec-collections's VecSet in AliasSet (noir-lang/noir#7058)
chore: reduce number of iterations of `acvm::compiler::compile` (noir-lang/noir#7050)
chore: add `noir_check_shuffle` as a critical library (noir-lang/noir#7056)
chore: clippy warning fix (noir-lang/noir#7051)
chore(ci): Unify compilation/execution report jobs that take averages with single runs  (noir-lang/noir#7048)
fix(nargo_fmt): let doc comment could come after regular comment (noir-lang/noir#7046)
fix(nargo_fmt): don't consider identifiers the same if they are equal… (noir-lang/noir#7043)
feat: auto-import traits when suggesting trait methods (noir-lang/noir#7037)
feat: avoid inserting `inc_rc` instructions into ACIR (noir-lang/noir#7036)
fix(lsp): suggest all possible trait methods, but only visible ones (noir-lang/noir#7027)
chore: add more protocol circuits to reports (noir-lang/noir#6977)
feat: avoid generating a new witness when checking if linear expression is zero (noir-lang/noir#7031)
feat: skip codegen of zero iteration loops (noir-lang/noir#7030)
TomAFrench added a commit to AztecProtocol/aztec-packages that referenced this pull request Jan 15, 2025
Automated pull of development from the
[noir](https://github.com/noir-lang/noir) programming language, a
dependency of Aztec.
BEGIN_COMMIT_OVERRIDE
chore: add end step for formatting workflow
(noir-lang/noir#7070)
feat(LSP): code action to import trait in a method call
(noir-lang/noir#7066)
feat!: Handle generic fields in `StructDefinition::fields` and move old
functionality to `StructDefinition::fields_as_written`
(noir-lang/noir#7067)
chore(ci): Check various inliner aggressiveness setttings in Brillig
reports (noir-lang/noir#7049)
chore: reenable reports on rollup root circuits
(noir-lang/noir#7061)
fix: don't always select trait impl when verifying trait constraints
(noir-lang/noir#7041)
chore: mark some critical libraries as good again
(noir-lang/noir#7065)
fix: Reduce memory usage in mem2reg
(noir-lang/noir#7053)
feat: Allow associated types to be ellided from trait constraints
(noir-lang/noir#7026)
chore(perf): try using vec-collections's VecSet in AliasSet
(noir-lang/noir#7058)
chore: reduce number of iterations of `acvm::compiler::compile`
(noir-lang/noir#7050)
chore: add `noir_check_shuffle` as a critical library
(noir-lang/noir#7056)
chore: clippy warning fix (noir-lang/noir#7051)
chore(ci): Unify compilation/execution report jobs that take averages
with single runs (noir-lang/noir#7048)
fix(nargo_fmt): let doc comment could come after regular comment
(noir-lang/noir#7046)
fix(nargo_fmt): don't consider identifiers the same if they are equal…
(noir-lang/noir#7043)
feat: auto-import traits when suggesting trait methods
(noir-lang/noir#7037)
feat: avoid inserting `inc_rc` instructions into ACIR
(noir-lang/noir#7036)
fix(lsp): suggest all possible trait methods, but only visible ones
(noir-lang/noir#7027)
chore: add more protocol circuits to reports
(noir-lang/noir#6977)
feat: avoid generating a new witness when checking if linear expression
is zero (noir-lang/noir#7031)
feat: skip codegen of zero iteration loops
(noir-lang/noir#7030)
END_COMMIT_OVERRIDE

---------

Co-authored-by: Tom French <tom@tomfren.ch>
Co-authored-by: Tom French <15848336+TomAFrench@users.noreply.github.com>
Co-authored-by: Jake Fecher <jfecher11@gmail.com>
AztecBot added a commit to AztecProtocol/aztec-nr that referenced this pull request Jan 16, 2025
Automated pull of development from the
[noir](https://github.com/noir-lang/noir) programming language, a
dependency of Aztec.
BEGIN_COMMIT_OVERRIDE
chore: add end step for formatting workflow
(noir-lang/noir#7070)
feat(LSP): code action to import trait in a method call
(noir-lang/noir#7066)
feat!: Handle generic fields in `StructDefinition::fields` and move old
functionality to `StructDefinition::fields_as_written`
(noir-lang/noir#7067)
chore(ci): Check various inliner aggressiveness setttings in Brillig
reports (noir-lang/noir#7049)
chore: reenable reports on rollup root circuits
(noir-lang/noir#7061)
fix: don't always select trait impl when verifying trait constraints
(noir-lang/noir#7041)
chore: mark some critical libraries as good again
(noir-lang/noir#7065)
fix: Reduce memory usage in mem2reg
(noir-lang/noir#7053)
feat: Allow associated types to be ellided from trait constraints
(noir-lang/noir#7026)
chore(perf): try using vec-collections's VecSet in AliasSet
(noir-lang/noir#7058)
chore: reduce number of iterations of `acvm::compiler::compile`
(noir-lang/noir#7050)
chore: add `noir_check_shuffle` as a critical library
(noir-lang/noir#7056)
chore: clippy warning fix (noir-lang/noir#7051)
chore(ci): Unify compilation/execution report jobs that take averages
with single runs (noir-lang/noir#7048)
fix(nargo_fmt): let doc comment could come after regular comment
(noir-lang/noir#7046)
fix(nargo_fmt): don't consider identifiers the same if they are equal…
(noir-lang/noir#7043)
feat: auto-import traits when suggesting trait methods
(noir-lang/noir#7037)
feat: avoid inserting `inc_rc` instructions into ACIR
(noir-lang/noir#7036)
fix(lsp): suggest all possible trait methods, but only visible ones
(noir-lang/noir#7027)
chore: add more protocol circuits to reports
(noir-lang/noir#6977)
feat: avoid generating a new witness when checking if linear expression
is zero (noir-lang/noir#7031)
feat: skip codegen of zero iteration loops
(noir-lang/noir#7030)
END_COMMIT_OVERRIDE

---------

Co-authored-by: Tom French <tom@tomfren.ch>
Co-authored-by: Tom French <15848336+TomAFrench@users.noreply.github.com>
Co-authored-by: Jake Fecher <jfecher11@gmail.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