chore(ssa): Turn the Brillig constraints check back on by default #7404
Conversation
|
@aakoshh, also changed tracking to employ a HashSet<(FunctionId, InstructionId)> instead of the count, as that one did in fact lead to some strange miscounts on tests with the check back on, and moved the tracking logic entirely out of TaintedIds to clean it up. Seems to be fine performance wise. |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Compilation Time
| Benchmark suite | Current: b36daf4 | Previous: fdd2fe7 | Ratio |
|---|---|---|---|
sha256_regression |
1.03 s |
0.971 s |
1.06 |
regression_4709 |
0.682 s |
0.716 s |
0.95 |
ram_blowup_regression |
20.7 s |
20.6 s |
1.00 |
global_var_regression_entry_points |
0.508 s |
0.497 s |
1.02 |
private-kernel-inner |
2.236 s |
2.102 s |
1.06 |
private-kernel-reset |
6.826 s |
6.588 s |
1.04 |
private-kernel-tail |
1.03 s |
1.011 s |
1.02 |
rollup-base-private |
8.866 s |
8.808 s |
1.01 |
rollup-base-public |
5.198 s |
5.15 s |
1.01 |
rollup-block-root-empty |
0.934 s |
0.946 s |
0.99 |
rollup-block-root-single-tx |
110 s |
97.7 s |
1.13 |
rollup-block-root |
107 s |
103 s |
1.04 |
rollup-merge |
0.928 s |
0.904 s |
1.03 |
rollup-root |
1.508 s |
1.636 s |
0.92 |
This comment was automatically generated by workflow using github-action-benchmark.
There was a problem hiding this comment.
Execution Time
| Benchmark suite | Current: b36daf4 | Previous: fdd2fe7 | Ratio |
|---|---|---|---|
private-kernel-inner |
0.07 s |
0.07 s |
1 |
private-kernel-reset |
0.29 s |
0.293 s |
0.99 |
private-kernel-tail |
0.017 s |
0.017 s |
1 |
rollup-base-private |
0.424 s |
0.427 s |
0.99 |
rollup-base-public |
0.18 s |
0.181 s |
0.99 |
rollup-block-root |
16.7 s |
16.7 s |
1 |
rollup-merge |
0.006 s |
0.006 s |
1 |
rollup-root |
0.025 s |
0.026 s |
0.96 |
This comment was automatically generated by workflow using github-action-benchmark.
There was a problem hiding this comment.
Test Suite Duration
| Benchmark suite | Current: b36daf4 | Previous: fdd2fe7 | Ratio |
|---|---|---|---|
AztecProtocol_aztec-packages_noir-projects_aztec-nr |
42 s |
43 s |
0.98 |
AztecProtocol_aztec-packages_noir-projects_noir-contracts |
77 s |
75 s |
1.03 |
AztecProtocol_aztec-packages_noir-projects_noir-protocol-circuits_crates_blob |
44 s |
47 s |
0.94 |
AztecProtocol_aztec-packages_noir-projects_noir-protocol-circuits_crates_private-kernel-lib |
164 s |
161 s |
1.02 |
AztecProtocol_aztec-packages_noir-projects_noir-protocol-circuits_crates_reset-kernel-lib |
11 s |
10 s |
1.10 |
AztecProtocol_aztec-packages_noir-projects_noir-protocol-circuits_crates_rollup-lib |
159 s |
166 s |
0.96 |
AztecProtocol_aztec-packages_noir-projects_noir-protocol-circuits_crates_types |
55 s |
55 s |
1 |
noir-lang_noir-bignum_ |
70 s |
74 s |
0.95 |
noir-lang_noir_bigcurve_ |
221 s |
255 s |
0.87 |
noir-lang_noir_json_parser_ |
9 s |
9 s |
1 |
This comment was automatically generated by workflow using github-action-benchmark.
There was a problem hiding this comment.
Execution Memory
| Benchmark suite | Current: b36daf4 | Previous: fdd2fe7 | Ratio |
|---|---|---|---|
private-kernel-inner |
244.52 MB |
244.52 MB |
1 |
private-kernel-reset |
278.76 MB |
278.76 MB |
1 |
private-kernel-tail |
214.09 MB |
214.09 MB |
1 |
rollup-base-private |
468.21 MB |
468.21 MB |
1 |
rollup-base-public |
409.26 MB |
409.26 MB |
1 |
rollup-block-root |
1420 MB |
1420 MB |
1 |
rollup-merge |
348.27 MB |
348.27 MB |
1 |
rollup-root |
355.26 MB |
355.26 MB |
1 |
This comment was automatically generated by workflow using github-action-benchmark.
There was a problem hiding this comment.
Compilation Memory
| Benchmark suite | Current: b36daf4 | Previous: fdd2fe7 | Ratio |
|---|---|---|---|
private-kernel-inner |
304.74 MB |
304.74 MB |
1 |
private-kernel-reset |
614.45 MB |
614.45 MB |
1 |
private-kernel-tail |
228.33 MB |
228.34 MB |
1.00 |
rollup-base-private |
978.09 MB |
978.09 MB |
1 |
rollup-base-public |
855.23 MB |
855.18 MB |
1.00 |
rollup-block-root-empty |
365.29 MB |
365.29 MB |
1 |
rollup-block-root-single-tx |
7870 MB |
7870 MB |
1 |
rollup-block-root |
7870 MB |
7870 MB |
1 |
rollup-merge |
363.69 MB |
363.7 MB |
1.00 |
rollup-root |
411.27 MB |
411.23 MB |
1.00 |
This comment was automatically generated by workflow using github-action-benchmark.
compiler/noirc_evaluator/src/ssa/checks/check_for_underconstrained_values.rs
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
⚠️ 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: 494e750 | Previous: 31cc6a1 | Ratio |
|---|---|---|---|
AztecProtocol_aztec-packages_noir-projects_noir-protocol-circuits_crates_blob |
64 s |
51 s |
1.25 |
This comment was automatically generated by workflow using github-action-benchmark.
CC: @TomAFrench
compiler/noirc_evaluator/src/ssa/checks/check_for_underconstrained_values.rs
Outdated
Show resolved
Hide resolved
compiler/noirc_evaluator/src/ssa/checks/check_for_underconstrained_values.rs
Outdated
Show resolved
Hide resolved
compiler/noirc_evaluator/src/ssa/checks/check_for_underconstrained_values.rs
Outdated
Show resolved
Hide resolved
compiler/noirc_evaluator/src/ssa/checks/check_for_underconstrained_values.rs
Outdated
Show resolved
Hide resolved
compiler/noirc_evaluator/src/ssa/checks/check_for_underconstrained_values.rs
Outdated
Show resolved
Hide resolved
…ined_values.rs Co-authored-by: Akosh Farkash <[email protected]>
…ined_values.rs Co-authored-by: Akosh Farkash <[email protected]>
There was a problem hiding this comment.
⚠️ Performance Alert ⚠️
Possible performance regression was detected for benchmark 'Compilation Time'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.20.
| Benchmark suite | Current: d0183a2 | Previous: ba66d3b | Ratio |
|---|---|---|---|
private-kernel-inner |
2.452 s |
2.03 s |
1.21 |
This comment was automatically generated by workflow using github-action-benchmark.
CC: @TomAFrench
github-actions
bot
added
the
documentation
|
🚀 Deployed on https://67c0e3d47ebdac29b0991633--noir-docs.netlify.app |
|
@TomAFrench is this okay to merge? |
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
|
Sure, I'm fine with this |
|
FYI @noir-lang/developerrelations on Noir doc changes. |
Description
As the reasons of the major compile time performance hit on certain large unrolled functions (e.g. rollup-block-root) have been figured out and eliminated in #7190 after temporarily switching the Brillig constraints check off by default in #7211, turn the check back on.
Problem
Summary
Reverts #7211
Additional Context
Documentation
Check one:
PR Checklist*
cargo fmton default settings.