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

chore: revert #6685 #6774

Closed
wants to merge 1 commit into from
Closed

chore: revert #6685 #6774

wants to merge 1 commit into from

Conversation

TomAFrench
Copy link
Member

Description

Problem*

Resolves

Summary*

This PR reverts the commit which contains #6685 due to the correctness issues identified in #6763

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 added the run-external-checks Trigger CI job to run tests on external repos label Dec 11, 2024
Copy link
Contributor

Changes to Brillig bytecode sizes

Generated at commit: d2816639721b972fde9772cc08e9b8fb5788bd17, compared to commit: e97339733000d19163372c76b299333c20db0299

🧾 Summary (10% most significant diffs)

Program Brillig opcodes (+/-) %
bench_2_to_17 +62 ❌ +19.08%
poseidon2 +59 ❌ +17.77%
fold_2_to_17 +98 ❌ +16.87%
brillig_rc_regression_6123 +26 ❌ +14.53%
no_predicates_numeric_generic_poseidon +102 ❌ +13.47%
fold_numeric_generic_poseidon +102 ❌ +13.47%
keccak256 -368 ✅ -16.75%
slices -541 ✅ -22.81%
reference_counts -170 ✅ -32.08%

Full diff report 👇
Program Brillig opcodes (+/-) %
bench_2_to_17 387 (+62) +19.08%
poseidon2 391 (+59) +17.77%
fold_2_to_17 679 (+98) +16.87%
brillig_rc_regression_6123 205 (+26) +14.53%
no_predicates_numeric_generic_poseidon 859 (+102) +13.47%
fold_numeric_generic_poseidon 859 (+102) +13.47%
regression_6674_3 671 (+68) +11.28%
wildcard_type 354 (+26) +7.93%
regression_6674_1 272 (+16) +6.25%
regression_6674_2 278 (+16) +6.11%
array_sort 310 (+14) +4.73%
nested_array_dynamic_simple 140 (+6) +4.48%
brillig_constant_reference_regression 90 (+3) +3.45%
conditional_2 119 (+3) +2.59%
regression_4202 125 (+3) +2.46%
conditional_regression_661 130 (+3) +2.36%
regression_struct_array_conditional 620 (+14) +2.31%
loop_invariant_regression 140 (+3) +2.19%
references 157 (+3) +1.95%
fold_call_witness_condition 114 (+2) +1.79%
fold_complex_outputs 639 (+11) +1.75%
brillig_cow_assign 140 (+2) +1.45%
uhashmap 14,378 (+200) +1.41%
pedersen_hash 375 (+5) +1.35%
array_dynamic_nested_blackbox_input 939 (+11) +1.19%
nested_dyn_array_regression_5782 177 (+2) +1.14%
slice_loop 281 (+3) +1.08%
array_dynamic_blackbox_input 1,055 (+11) +1.05%
pedersen_commitment 211 (+2) +0.96%
databus_two_calldata 213 (+2) +0.95%
to_be_bytes 214 (+2) +0.94%
conditional_1 1,217 (+11) +0.91%
ecdsa_secp256k1 934 (+8) +0.86%
brillig_nested_arrays 383 (+3) +0.79%
array_dynamic 321 (+2) +0.63%
regression_4709 134,511 (+771) +0.58%
regression 959 (+5) +0.52%
modulus 1,774 (+6) +0.34%
slice_dynamic_index 2,524 (+2) +0.08%
sha2_byte 2,786 (+1) +0.04%
brillig_cow_regression 2,204 (-2) -0.09%
pedersen_check 573 (-1) -0.17%
brillig_pedersen 573 (-1) -0.17%
nested_array_in_slice 1,418 (-3) -0.21%
nested_array_dynamic 2,771 (-7) -0.25%
brillig_cow 386 (-1) -0.26%
u128 2,807 (-10) -0.35%
regression_5252 4,708 (-18) -0.38%
poseidon_bn254_hash 5,483 (-22) -0.40%
poseidon_bn254_hash_width_3 5,483 (-22) -0.40%
higher_order_functions 692 (-3) -0.43%
hashmap 21,743 (-156) -0.71%
array_if_cond_simple 131 (-1) -0.76%
sha256_brillig_performance_regression 1,708 (-14) -0.81%
brillig_arrays 114 (-1) -0.87%
regression_mem_op_predicate 109 (-1) -0.91%
struct_inputs 276 (-3) -1.08%
array_dynamic_main_output 91 (-1) -1.09%
debug_logs 5,085 (-75) -1.45%
poseidonsponge_x5_254 4,283 (-70) -1.61%
slice_regex 2,566 (-63) -2.40%
simple_shield 892 (-22) -2.41%
brillig_oracle 361 (-9) -2.43%
conditional_regression_short_circuit 1,268 (-32) -2.46%
6 1,192 (-35) -2.85%
array_to_slice 712 (-21) -2.86%
strings 914 (-27) -2.87%
sha256_var_witness_const_regression 1,295 (-43) -3.21%
aes128_encrypt 535 (-20) -3.60%
sha256_var_size_regression 1,800 (-68) -3.64%
merkle_insert 776 (-32) -3.96%
sha256_regression 6,733 (-280) -3.99%
sha256 2,348 (-132) -5.32%
sha256_var_padding_regression 4,927 (-283) -5.43%
7_function 546 (-36) -6.19%
bigint 2,104 (-163) -7.19%
keccak256 1,829 (-368) -16.75%
slices 1,831 (-541) -22.81%
reference_counts 360 (-170) -32.08%

Copy link
Contributor

Changes to number of Brillig opcodes executed

Generated at commit: d2816639721b972fde9772cc08e9b8fb5788bd17, compared to commit: e97339733000d19163372c76b299333c20db0299

🧾 Summary (10% most significant diffs)

Program Brillig opcodes (+/-) %
pedersen_hash +141 ❌ +26.65%
brillig_pedersen +178 ❌ +22.25%
pedersen_check +178 ❌ +22.25%
simple_shield +453 ❌ +18.88%
pedersen_commitment +46 ❌ +18.47%
merkle_insert +586 ❌ +18.34%
fold_2_to_17 +198,851 ❌ +18.18%
bench_2_to_17 +104,559 ❌ +17.73%
reference_counts -133 ✅ -29.10%

Full diff report 👇
Program Brillig opcodes (+/-) %
pedersen_hash 670 (+141) +26.65%
brillig_pedersen 978 (+178) +22.25%
pedersen_check 978 (+178) +22.25%
simple_shield 2,853 (+453) +18.88%
pedersen_commitment 295 (+46) +18.47%
merkle_insert 3,781 (+586) +18.34%
fold_2_to_17 1,292,773 (+198,851) +18.18%
bench_2_to_17 694,397 (+104,559) +17.73%
fold_numeric_generic_poseidon 5,862 (+855) +17.08%
no_predicates_numeric_generic_poseidon 5,862 (+855) +17.08%
poseidon2 803 (+108) +15.54%
array_sort 607 (+81) +15.40%
wildcard_type 592 (+70) +13.41%
regression_6674_3 1,812 (+200) +12.41%
brillig_rc_regression_6123 335 (+36) +12.04%
databus_two_calldata 445 (+46) +11.53%
regression_struct_array_conditional 1,710 (+165) +10.68%
to_be_bytes 2,454 (+235) +10.59%
conditional_1 5,909 (+484) +8.92%
brillig_cow_regression 520,383 (+40,946) +8.54%
ram_blowup_regression 780,775 (+60,552) +8.41%
regression_6674_1 896 (+68) +8.21%
brillig_oracle 2,624 (+185) +7.59%
sha256_var_padding_regression 220,289 (+15,438) +7.54%
regression_6674_2 884 (+50) +6.00%
7_function 2,476 (+131) +5.59%
array_dynamic_nested_blackbox_input 4,753 (+246) +5.46%
nested_array_dynamic_simple 126 (+6) +5.00%
fold_call_witness_condition 70 (+3) +4.48%
aes128_encrypt 4,514 (+176) +4.06%
array_dynamic_blackbox_input 18,587 (+675) +3.77%
ecdsa_secp256k1 6,982 (+245) +3.64%
brillig_constant_reference_regression 87 (+3) +3.57%
sha2_byte 46,959 (+1,423) +3.13%
uhashmap 151,372 (+4,424) +3.01%
sha256_var_witness_const_regression 7,017 (+183) +2.68%
higher_order_functions 1,269 (+33) +2.67%
conditional_2 122 (+3) +2.52%
conditional_regression_661 123 (+3) +2.50%
6 7,502 (+156) +2.12%
conditional_regression_short_circuit 7,580 (+156) +2.10%
keccak256 33,164 (+579) +1.78%
sha256_brillig_performance_regression 23,107 (+361) +1.59%
regression_4202 214 (+3) +1.42%
references 239 (+3) +1.27%
nested_dyn_array_regression_5782 176 (+2) +1.15%
fold_complex_outputs 922 (+10) +1.10%
brillig_nested_arrays 356 (+3) +0.85%
nested_array_dynamic 3,746 (+26) +0.70%
u128 25,012 (+118) +0.47%
array_dynamic 500 (+2) +0.40%
slice_loop 960 (+3) +0.31%
sha256_var_size_regression 16,658 (+44) +0.26%
loop_invariant_regression 1,167 (+3) +0.26%
regression 2,831 (+6) +0.21%
brillig_cow_assign 534 (+1) +0.19%
hashmap 55,604 (+64) +0.12%
slice_dynamic_index 4,684 (+2) +0.04%
modulus 19,184 (+6) +0.03%
sha256 14,534 (+2) +0.01%
poseidonsponge_x5_254 184,085 (-61) -0.03%
brillig_cow 1,153 (-2) -0.17%
sha256_regression 116,973 (-207) -0.18%
array_if_cond_simple 537 (-1) -0.19%
regression_5252 917,714 (-2,300) -0.25%
array_dynamic_main_output 296 (-1) -0.34%
nested_array_in_slice 1,735 (-6) -0.34%
struct_inputs 584 (-3) -0.51%
brillig_arrays 139 (-1) -0.71%
array_to_slice 1,664 (-21) -1.25%
debug_logs 5,097 (-75) -1.45%
strings 1,767 (-27) -1.51%
slice_regex 3,798 (-63) -1.63%
poseidon_bn254_hash 162,947 (-3,593) -2.16%
poseidon_bn254_hash_width_3 162,947 (-3,593) -2.16%
regression_4449 214,164 (-7,128) -3.22%
slices 3,136 (-358) -10.25%
reference_counts 324 (-133) -29.10%

Copy link
Contributor

Changes to circuit sizes

Generated at commit: d2816639721b972fde9772cc08e9b8fb5788bd17, compared to commit: e97339733000d19163372c76b299333c20db0299

🧾 Summary (10% most significant diffs)

Program ACIR opcodes (+/-) % Circuit size (+/-) %
reference_counts -4 ✅ -57.14% 0 ➖ 0.00%
hashmap -18,650 ✅ -37.87% -43,024 ✅ -38.07%

Full diff report 👇
Program ACIR opcodes (+/-) % Circuit size (+/-) %
reference_counts 3 (-4) -57.14% 16 (0) 0.00%
hashmap 30,599 (-18,650) -37.87% 69,989 (-43,024) -38.07%

Copy link
Contributor

Peak Memory Sample

Program Peak Memory %
keccak256 82.02M 1%
workspace 121.87M -1%
regression_4709 327.82M 0%
ram_blowup_regression 2.54G 0%
private-kernel-tail 231.79M 0%
private-kernel-reset 1.37G 0%
private-kernel-inner 338.30M 0%
parity-root 199.34M 0%

Copy link
Contributor

Compilation Sample

Program Compilation Time %
sha256_regression 0m1.616s 10%
regression_4709 0m1.535s -4%
ram_blowup_regression 0m17.064s -1%
private-kernel-tail 0m1.224s 0%
private-kernel-reset 0m10.015s 4%
private-kernel-inner 0m2.474s -7%
parity-root 0m1.196s 35%
noir-contracts 2m49.183s 0%

@jfecher
Copy link
Contributor

jfecher commented Dec 11, 2024

Closing this in favor of #6778

@jfecher jfecher closed this Dec 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
run-external-checks Trigger CI job to run tests on external repos
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants