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: Test RcTracker with starting inc_rc elision re-added #6797

Draft
wants to merge 2 commits into
base: mv/bring-back-rc-tracker
Choose a base branch
from

Conversation

jfecher
Copy link
Contributor

@jfecher jfecher commented Dec 12, 2024

Description

Problem*

Resolves

Summary*

Fork of #6783 to test it on external repos with the change to start reference counts at 2 reverted to start at 1 again.

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.

@jfecher jfecher added the run-external-checks Trigger CI job to run tests on external repos label Dec 12, 2024
@jfecher jfecher changed the title Re-add should_inc_rc chore: Test RcTracker with starting inc_rc elision re-added Dec 12, 2024
Copy link
Contributor

github-actions bot commented Dec 12, 2024

Changes to Brillig bytecode sizes

Generated at commit: b1f1d3388052ff162fe979a409dd48a749803002, compared to commit: f597b976a6a324cee9bc327b73d40211be05903e

🧾 Summary (10% most significant diffs)

Program Brillig opcodes (+/-) %
simple_shield -15 ✅ -1.66%
sha256_var_padding_regression -90 ✅ -1.75%
keccak256 -39 ✅ -1.78%
brillig_cow_assign -3 ✅ -2.13%
conditional_regression_661 -3 ✅ -2.36%
regression_4202 -3 ✅ -2.40%
fold_call_witness_condition -3 ✅ -2.61%

Full diff report 👇
Program Brillig opcodes (+/-) %
u128 2,754 (-3) -0.11%
slice_dynamic_index 2,504 (-3) -0.12%
nested_array_dynamic 2,454 (-3) -0.12%
strings 914 (-3) -0.33%
array_to_slice 715 (-3) -0.42%
higher_order_functions 674 (-3) -0.44%
debug_logs 5,124 (-24) -0.47%
poseidon_bn254_hash 5,424 (-27) -0.50%
poseidon_bn254_hash_width_3 5,424 (-27) -0.50%
regression_6674_3 552 (-3) -0.54%
poseidonsponge_x5_254 4,284 (-24) -0.56%
regression 939 (-6) -0.63%
regression_5252 4,651 (-30) -0.64%
hashmap 21,662 (-147) -0.67%
poseidon2 332 (-3) -0.90%
sha256_brillig_performance_regression 1,647 (-15) -0.90%
bench_2_to_17 325 (-3) -0.91%
uhashmap 13,884 (-132) -0.94%
ecdsa_secp256k1 899 (-9) -0.99%
array_dynamic_nested_blackbox_input 892 (-9) -1.00%
wildcard_type 295 (-3) -1.01%
conditional_1 1,176 (-12) -1.01%
fold_2_to_17 578 (-6) -1.03%
regression_struct_array_conditional 570 (-6) -1.04%
slice_loop 266 (-3) -1.12%
sha256_regression 6,839 (-78) -1.13%
array_dynamic_blackbox_input 1,020 (-12) -1.16%
sha2_byte 2,734 (-33) -1.19%
regression_6674_2 247 (-3) -1.20%
regression_6674_1 244 (-3) -1.21%
brillig_cow_regression 2,140 (-27) -1.25%
fold_complex_outputs 475 (-6) -1.25%
reference_counts 467 (-6) -1.27%
bigint 2,165 (-30) -1.37%
slices 2,156 (-30) -1.37%
databus_two_calldata 211 (-3) -1.40%
to_be_bytes 209 (-3) -1.42%
conditional_regression_short_circuit 1,246 (-18) -1.42%
pedersen_commitment 206 (-3) -1.44%
merkle_insert 793 (-12) -1.49%
6 1,173 (-18) -1.51%
ram_blowup_regression 954 (-15) -1.55%
7_function 570 (-9) -1.55%
no_predicates_numeric_generic_poseidon 748 (-12) -1.58%
fold_numeric_generic_poseidon 748 (-12) -1.58%
pedersen_check 556 (-9) -1.59%
brillig_pedersen 556 (-9) -1.59%
regression_4449 739 (-12) -1.60%
sha256_var_witness_const_regression 1,287 (-21) -1.61%
pedersen_hash 367 (-6) -1.61%
sha256 2,369 (-39) -1.62%
brillig_oracle 364 (-6) -1.62%
aes128_encrypt 543 (-9) -1.63%
sha256_var_size_regression 1,787 (-30) -1.65%
simple_shield 890 (-15) -1.66%
sha256_var_padding_regression 5,039 (-90) -1.75%
keccak256 2,158 (-39) -1.78%
brillig_cow_assign 138 (-3) -2.13%
conditional_regression_661 124 (-3) -2.36%
regression_4202 122 (-3) -2.40%
fold_call_witness_condition 112 (-3) -2.61%

Copy link
Contributor

github-actions bot commented Dec 12, 2024

Peak Memory Sample

Program Peak Memory %
keccak256 79.14M 0%
workspace 121.88M -1%
regression_4709 295.98M 0%
ram_blowup_regression 2.44G 0%
private-kernel-tail 210.39M 0%
private-kernel-reset 860.67M -1%
private-kernel-inner 307.77M 0%
parity-root 175.77M 0%

Copy link
Contributor

github-actions bot commented Dec 12, 2024

Compilation Sample

Program Compilation Time %
sha256_regression 0m1.506s 4%
regression_4709 0m0.803s 7%
ram_blowup_regression 0m17.480s 3%
private-kernel-tail 0m1.131s -2%
private-kernel-reset 0m9.351s 2%
private-kernel-inner 0m2.529s 6%
parity-root 0m0.886s 0%
noir-contracts 2m37.104s -4%

@vezenovm
Copy link
Contributor

Fork of #6783 to test it on external repos with the change to start reference counts at 2 reverted to start at 1 again.

Looks like the first assert in reference_counts needs to be updated

Copy link
Contributor

Changes to number of Brillig opcodes executed

Generated at commit: b1f1d3388052ff162fe979a409dd48a749803002, compared to commit: f597b976a6a324cee9bc327b73d40211be05903e

🧾 Summary (10% most significant diffs)

Program Brillig opcodes (+/-) %
brillig_oracle -318 ✅ -11.56%
pedersen_commitment -47 ✅ -16.04%
brillig_pedersen -190 ✅ -19.55%
pedersen_check -190 ✅ -19.55%
simple_shield -584 ✅ -19.81%
merkle_insert -858 ✅ -21.34%
pedersen_hash -143 ✅ -21.38%

Full diff report 👇
Program Brillig opcodes (+/-) %
bench_2_to_17 589,838 (-3) -0.00%
fold_2_to_17 1,093,919 (-6) -0.00%
slice_dynamic_index 4,664 (-3) -0.06%
strings 1,767 (-3) -0.17%
array_to_slice 1,667 (-3) -0.18%
regression 2,810 (-6) -0.21%
slice_loop 939 (-3) -0.32%
regression_6674_2 819 (-3) -0.36%
regression_6674_1 816 (-3) -0.37%
poseidon_bn254_hash 165,946 (-689) -0.41%
poseidon_bn254_hash_width_3 165,946 (-689) -0.41%
hashmap 55,279 (-237) -0.43%
poseidon2 695 (-3) -0.43%
debug_logs 5,136 (-24) -0.47%
poseidonsponge_x5_254 183,546 (-864) -0.47%
regression_5252 916,132 (-4,366) -0.47%
u128 24,831 (-131) -0.52%
brillig_cow_assign 533 (-3) -0.56%
uhashmap 146,378 (-930) -0.63%
fold_complex_outputs 762 (-6) -0.78%
slices 3,335 (-30) -0.89%
nested_array_dynamic 3,417 (-47) -1.36%
regression_4202 211 (-3) -1.40%
reference_counts 397 (-6) -1.49%
conditional_regression_661 117 (-3) -2.50%
sha2_byte 45,276 (-1,435) -3.07%
higher_order_functions 1,215 (-40) -3.19%
regression_6674_3 1,561 (-54) -3.34%
ecdsa_secp256k1 6,540 (-249) -3.67%
sha256_brillig_performance_regression 22,611 (-904) -3.84%
fold_numeric_generic_poseidon 4,935 (-205) -3.99%
no_predicates_numeric_generic_poseidon 4,935 (-205) -3.99%
sha256_regression 116,354 (-4,918) -4.06%
sha256_var_size_regression 16,315 (-714) -4.19%
fold_call_witness_condition 67 (-3) -4.29%
array_dynamic_nested_blackbox_input 4,301 (-249) -5.47%
conditional_regression_short_circuit 7,030 (-498) -6.62%
keccak256 32,474 (-2,303) -6.62%
6 6,952 (-498) -6.68%
sha256 13,869 (-1,127) -7.52%
array_dynamic_blackbox_input 17,524 (-1,432) -7.55%
regression_4449 207,712 (-17,433) -7.74%
7_function 2,333 (-204) -8.04%
aes128_encrypt 4,326 (-393) -8.33%
conditional_1 5,225 (-492) -8.61%
sha256_var_witness_const_regression 6,571 (-629) -8.74%
wildcard_type 471 (-47) -9.07%
ram_blowup_regression 717,859 (-74,380) -9.39%
brillig_cow_regression 477,835 (-50,025) -9.48%
to_be_bytes 2,216 (-236) -9.62%
regression_struct_array_conditional 1,515 (-164) -9.77%
sha256_var_padding_regression 204,188 (-22,308) -9.85%
databus_two_calldata 399 (-47) -10.54%
brillig_oracle 2,433 (-318) -11.56%
pedersen_commitment 246 (-47) -16.04%
brillig_pedersen 782 (-190) -19.55%
pedersen_check 782 (-190) -19.55%
simple_shield 2,364 (-584) -19.81%
merkle_insert 3,162 (-858) -21.34%
pedersen_hash 526 (-143) -21.38%

@TomAFrench
Copy link
Member

Seems like we're getting correctness issues inside of bigcurve again.

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.

3 participants