Skip to content

feat(experimental): Always inline certain methods in sha#7364

Closed
vezenovm wants to merge 2 commits intomasterfrom
mv/inline-always-certain-sha-methods
Closed

feat(experimental): Always inline certain methods in sha#7364
vezenovm wants to merge 2 commits intomasterfrom
mv/inline-always-certain-sha-methods

Conversation

@vezenovm
Copy link
Contributor

Description

Problem*

Optimizing bytecode sizes and execution traces in Brillig

Summary*

I know we are deprecating sha256 in the stdlib and moving to use noir-lang/sha256. I have replicated this PR here noir-lang/sha256#13. I am just pushing this draft for now as we have more complete benchmarking and visualization on this repo.

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.

@github-actions
Copy link
Contributor

github-actions bot commented Feb 13, 2025

Changes to number of Brillig opcodes executed

Generated at commit: 60164846b3715aedfc09fc93e2d5a84d445b5621, compared to commit: 97afa52f5212be2d05af26b9e8dde9c3ea7a1d2e

🧾 Summary (10% most significant diffs)

Program Brillig opcodes (+/-) %
sha256_var_size_regression_inliner_zero -5,164 ✅ -24.23%
sha256_var_padding_regression_inliner_min -57,130 ✅ -24.67%
sha256_var_padding_regression_inliner_zero -56,770 ✅ -24.67%
sha256_regression_inliner_min -42,603 ✅ -27.10%
sha256_regression_inliner_zero -42,191 ✅ -27.19%

Full diff report 👇
Program Brillig opcodes (+/-) %
sha2_byte_inliner_min 88,444 (-329) -0.37%
sha2_byte_inliner_zero 71,391 (-280) -0.39%
array_dynamic_nested_blackbox_input_inliner_zero 4,199 (-252) -5.66%
array_dynamic_nested_blackbox_input_inliner_min 4,607 (-298) -6.08%
sha256_inliner_zero 10,547 (-700) -6.22%
sha256_var_witness_const_regression_inliner_zero 6,431 (-448) -6.51%
conditional_1_inliner_zero 5,088 (-381) -6.97%
conditional_1_inliner_min 5,509 (-429) -7.22%
sha256_inliner_min 11,274 (-909) -7.46%
sha256_var_witness_const_regression_inliner_min 7,092 (-580) -7.56%
conditional_regression_short_circuit_inliner_zero 4,110 (-370) -8.26%
6_inliner_zero 4,030 (-370) -8.41%
conditional_regression_short_circuit_inliner_min 4,490 (-429) -8.72%
6_inliner_min 4,384 (-429) -8.91%
regression_4449_inliner_min 235,513 (-29,820) -11.24%
regression_4449_inliner_zero 198,065 (-26,042) -11.62%
ecdsa_secp256k1_inliner_min 6,908 (-1,241) -15.23%
ecdsa_secp256k1_inliner_zero 6,378 (-1,192) -15.75%
brillig_cow_regression_inliner_min 456,919 (-86,030) -15.84%
brillig_cow_regression_inliner_zero 453,386 (-85,966) -15.94%
ram_blowup_regression_inliner_min 685,985 (-131,322) -16.07%
ram_blowup_regression_inliner_zero 681,794 (-131,255) -16.14%
array_dynamic_blackbox_input_inliner_min 18,600 (-3,664) -16.46%
array_dynamic_blackbox_input_inliner_zero 17,644 (-3,533) -16.68%
sha256_brillig_performance_regression_inliner_zero 21,526 (-4,369) -16.87%
sha256_brillig_performance_regression_inliner_min 21,842 (-4,434) -16.87%
sha256_var_size_regression_inliner_min 16,784 (-5,282) -23.94%
sha256_var_size_regression_inliner_zero 16,147 (-5,164) -24.23%
sha256_var_padding_regression_inliner_min 174,492 (-57,130) -24.67%
sha256_var_padding_regression_inliner_zero 173,353 (-56,770) -24.67%
sha256_regression_inliner_min 114,591 (-42,603) -27.10%
sha256_regression_inliner_zero 112,961 (-42,191) -27.19%

@github-actions
Copy link
Contributor

github-actions bot commented Feb 13, 2025

Changes to Brillig bytecode sizes

Generated at commit: 60164846b3715aedfc09fc93e2d5a84d445b5621, compared to commit: 97afa52f5212be2d05af26b9e8dde9c3ea7a1d2e

🧾 Summary (10% most significant diffs)

Program Brillig opcodes (+/-) %
sha256_var_witness_const_regression_inliner_zero -14 ✅ -1.76%
conditional_1_inliner_zero -27 ✅ -2.44%
ecdsa_secp256k1_inliner_zero -27 ✅ -3.00%
array_dynamic_nested_blackbox_input_inliner_zero -27 ✅ -3.08%
regression_4449_inliner_zero -37 ✅ -4.89%

Full diff report 👇
Program Brillig opcodes (+/-) %
sha256_regression_inliner_zero 4,829 (-6) -0.12%
sha256_var_padding_regression_inliner_zero 2,910 (-6) -0.21%
sha256_regression_inliner_min 5,256 (-17) -0.32%
sha256_inliner_zero 1,226 (-6) -0.49%
sha256_var_padding_regression_inliner_min 3,175 (-17) -0.53%
sha2_byte_inliner_min 2,946 (-17) -0.57%
brillig_cow_regression_inliner_min 2,314 (-17) -0.73%
brillig_cow_regression_inliner_zero 2,059 (-16) -0.77%
sha256_brillig_performance_regression_inliner_zero 1,641 (-15) -0.91%
sha256_brillig_performance_regression_inliner_min 1,791 (-17) -0.94%
conditional_1_inliner_min 1,390 (-17) -1.21%
sha256_inliner_min 1,368 (-17) -1.23%
sha2_byte_inliner_zero 2,152 (-27) -1.24%
sha256_var_size_regression_inliner_zero 1,071 (-14) -1.29%
array_dynamic_blackbox_input_inliner_min 1,244 (-17) -1.35%
sha256_var_size_regression_inliner_min 1,224 (-17) -1.37%
array_dynamic_blackbox_input_inliner_zero 994 (-14) -1.39%
ecdsa_secp256k1_inliner_min 1,189 (-17) -1.41%
ram_blowup_regression_inliner_zero 908 (-13) -1.41%
array_dynamic_nested_blackbox_input_inliner_min 1,176 (-17) -1.42%
ram_blowup_regression_inliner_min 1,097 (-17) -1.53%
conditional_regression_short_circuit_inliner_min 1,087 (-17) -1.54%
conditional_regression_short_circuit_inliner_zero 894 (-14) -1.54%
regression_4449_inliner_min 1,042 (-17) -1.61%
6_inliner_min 986 (-17) -1.69%
6_inliner_zero 812 (-14) -1.69%
sha256_var_witness_const_regression_inliner_min 960 (-17) -1.74%
sha256_var_witness_const_regression_inliner_zero 780 (-14) -1.76%
conditional_1_inliner_zero 1,079 (-27) -2.44%
ecdsa_secp256k1_inliner_zero 874 (-27) -3.00%
array_dynamic_nested_blackbox_input_inliner_zero 851 (-27) -3.08%
regression_4449_inliner_zero 720 (-37) -4.89%

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

⚠️ Performance Alert ⚠️

Possible performance regression was detected for benchmark 'Test Suite Duration'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.20.

Benchmark suite Current: 99beecc Previous: 97afa52 Ratio
noir-lang_noir_bigcurve_ 311 s 252 s 1.23

This comment was automatically generated by workflow using github-action-benchmark.

CC: @TomAFrench

@vezenovm
Copy link
Contributor Author

Closing as noir-lang/sha256#13 is merged

@vezenovm vezenovm closed this Feb 13, 2025
@TomAFrench
Copy link
Member

I've rolled these changes into the 0.1.2 release which is now being used in Aztec in AztecProtocol/aztec-packages#11696

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