-
Notifications
You must be signed in to change notification settings - Fork 329
chore(opt): Fetch set of Brillig entry points without reachability and recursive functions computation #9844
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(opt): Fetch set of Brillig entry points without reachability and recursive functions computation #9844
Conversation
…brillig entry points
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ACVM Benchmarks
Benchmark suite | Current: a9a32e8 | Previous: 72c8262 | Ratio |
---|---|---|---|
purely_sequential_opcodes |
256969 ns/iter (± 1454 ) |
255274 ns/iter (± 569 ) |
1.01 |
perfectly_parallel_opcodes |
228246 ns/iter (± 6277 ) |
226675 ns/iter (± 6151 ) |
1.01 |
perfectly_parallel_batch_inversion_opcodes |
2794500 ns/iter (± 3573 ) |
2793380 ns/iter (± 6260 ) |
1.00 |
This comment was automatically generated by workflow using github-action-benchmark.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Artifact Size
Benchmark suite | Current: a9a32e8 | Previous: 72c8262 | Ratio |
---|---|---|---|
private-kernel-inner |
711.9 KB |
711.9 KB |
1 |
private-kernel-reset |
2027.3 KB |
2027.3 KB |
1 |
private-kernel-tail |
534.7 KB |
534.7 KB |
1 |
rollup-base-private |
4352.4 KB |
4352.4 KB |
1 |
rollup-base-public |
3359.4 KB |
3359.4 KB |
1 |
rollup-block-root-empty |
3861.1 KB |
3861.1 KB |
1 |
rollup-block-root-single-tx |
30912.3 KB |
30912.3 KB |
1 |
rollup-block-root |
30962.8 KB |
30962.8 KB |
1 |
rollup-merge |
190.2 KB |
190.2 KB |
1 |
rollup-root |
392.4 KB |
392.4 KB |
1 |
semaphore-depth-10 |
634 KB |
634 KB |
1 |
sha512-100-bytes |
527.6 KB |
527.6 KB |
1 |
This comment was automatically generated by workflow using github-action-benchmark.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Compilation Time
Benchmark suite | Current: a9a32e8 | Previous: 72c8262 | Ratio |
---|---|---|---|
private-kernel-inner |
1.886 s |
1.788 s |
1.05 |
private-kernel-reset |
8.228 s |
8.29 s |
0.99 |
private-kernel-tail |
1.362 s |
1.366 s |
1.00 |
rollup-base-private |
17.86 s |
18.56 s |
0.96 |
rollup-base-public |
16.56 s |
17 s |
0.97 |
rollup-block-root-empty |
17.56 s |
17.72 s |
0.99 |
rollup-block-root-single-tx |
203 s |
199 s |
1.02 |
rollup-block-root |
204 s |
207 s |
0.99 |
rollup-merge |
1.454 s |
1.37 s |
1.06 |
rollup-root |
1.488 s |
1.504 s |
0.99 |
semaphore-depth-10 |
0.821 s |
0.779 s |
1.05 |
sha512-100-bytes |
1.778 s |
1.901 s |
0.94 |
This comment was automatically generated by workflow using github-action-benchmark.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Opcode count
Benchmark suite | Current: a9a32e8 | Previous: 72c8262 | Ratio |
---|---|---|---|
private-kernel-inner |
14792 opcodes |
14792 opcodes |
1 |
private-kernel-reset |
68868 opcodes |
68868 opcodes |
1 |
private-kernel-tail |
11177 opcodes |
11177 opcodes |
1 |
rollup-base-private |
221339 opcodes |
221339 opcodes |
1 |
rollup-base-public |
159930 opcodes |
159930 opcodes |
1 |
rollup-block-root-empty |
67868 opcodes |
67868 opcodes |
1 |
rollup-block-root-single-tx |
963687 opcodes |
963687 opcodes |
1 |
rollup-block-root |
964973 opcodes |
964973 opcodes |
1 |
rollup-merge |
1409 opcodes |
1409 opcodes |
1 |
rollup-root |
2619 opcodes |
2619 opcodes |
1 |
semaphore-depth-10 |
5700 opcodes |
5700 opcodes |
1 |
sha512-100-bytes |
13173 opcodes |
13173 opcodes |
1 |
This comment was automatically generated by workflow using github-action-benchmark.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Execution Time
Benchmark suite | Current: a9a32e8 | Previous: 72c8262 | Ratio |
---|---|---|---|
private-kernel-inner |
0.017 s |
0.015 s |
1.13 |
private-kernel-reset |
0.161 s |
0.163 s |
0.99 |
private-kernel-tail |
0.011 s |
0.011 s |
1 |
rollup-base-private |
0.265 s |
0.265 s |
1 |
rollup-base-public |
0.165 s |
0.161 s |
1.02 |
rollup-block-root |
14.6 s |
14.4 s |
1.01 |
rollup-merge |
0.002 s |
0.002 s |
1 |
rollup-root |
0.004 s |
0.004 s |
1 |
semaphore-depth-10 |
0.019 s |
0.019 s |
1 |
sha512-100-bytes |
0.1 s |
0.109 s |
0.92 |
This comment was automatically generated by workflow using github-action-benchmark.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Compilation Memory
Benchmark suite | Current: a9a32e8 | Previous: 72c8262 | Ratio |
---|---|---|---|
private-kernel-inner |
285.36 MB |
285.31 MB |
1.00 |
private-kernel-reset |
594.36 MB |
594.36 MB |
1 |
private-kernel-tail |
259.93 MB |
259.93 MB |
1 |
rollup-base-private |
1360 MB |
1360 MB |
1 |
rollup-base-public |
1420 MB |
1420 MB |
1 |
rollup-block-root-empty |
1020 MB |
1020 MB |
1 |
rollup-block-root-single-tx |
9690 MB |
9690 MB |
1 |
rollup-block-root |
9690 MB |
9690 MB |
1 |
rollup-merge |
333.56 MB |
333.56 MB |
1 |
rollup-root |
344.28 MB |
344.28 MB |
1 |
semaphore_depth_10 |
107.77 MB |
107.77 MB |
1 |
sha512_100_bytes |
265.72 MB |
265.7 MB |
1.00 |
This comment was automatically generated by workflow using github-action-benchmark.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Test Suite Duration
Benchmark suite | Current: a9a32e8 | Previous: 72c8262 | Ratio |
---|---|---|---|
test_report_AztecProtocol_aztec-packages_noir-projects_aztec-nr |
100 s |
102 s |
0.98 |
test_report_AztecProtocol_aztec-packages_noir-projects_noir-contracts |
132 s |
130 s |
1.02 |
test_report_AztecProtocol_aztec-packages_noir-projects_noir-protocol-circuits_crates_blob |
203 s |
198 s |
1.03 |
test_report_AztecProtocol_aztec-packages_noir-projects_noir-protocol-circuits_crates_private-kernel-lib |
224 s |
217 s |
1.03 |
test_report_AztecProtocol_aztec-packages_noir-projects_noir-protocol-circuits_crates_reset-kernel-lib |
33 s |
36 s |
0.92 |
test_report_AztecProtocol_aztec-packages_noir-projects_noir-protocol-circuits_crates_rollup-lib |
629 s |
612 s |
1.03 |
test_report_AztecProtocol_aztec-packages_noir-projects_noir-protocol-circuits_crates_types |
99 s |
98 s |
1.01 |
test_report_noir-lang_noir-bignum_ |
130 s |
132 s |
0.98 |
test_report_noir-lang_noir_bigcurve_ |
394 s |
330 s |
1.19 |
test_report_noir-lang_sha256_ |
16 s |
15 s |
1.07 |
test_report_noir-lang_sha512_ |
13 s |
13 s |
1 |
test_report_zkpassport_noir-ecdsa_ |
2 s |
2 s |
1 |
test_report_zkpassport_noir_rsa_ |
1 s |
1 s |
1 |
This comment was automatically generated by workflow using github-action-benchmark.
There was a problem hiding this 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: b336ed1 | Previous: 00f5ece | Ratio |
---|---|---|---|
test_report_AztecProtocol_aztec-packages_noir-projects_noir-protocol-circuits_crates_blob |
277 s |
223 s |
1.24 |
test_report_zkpassport_noir_rsa_ |
2 s |
1 s |
2 |
This comment was automatically generated by workflow using github-action-benchmark.
CC: @TomAFrench
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Execution Memory
Benchmark suite | Current: a9a32e8 | Previous: 72c8262 | Ratio |
---|---|---|---|
private-kernel-inner |
258.97 MB |
258.97 MB |
1 |
private-kernel-reset |
292.14 MB |
292.14 MB |
1 |
private-kernel-tail |
243.75 MB |
243.75 MB |
1 |
rollup-base-private |
506.86 MB |
506.86 MB |
1 |
rollup-base-public |
439.27 MB |
439.27 MB |
1 |
rollup-block-root |
1540 MB |
1540 MB |
1 |
rollup-merge |
331.31 MB |
331.31 MB |
1 |
rollup-root |
333.83 MB |
333.83 MB |
1 |
semaphore_depth_10 |
73 MB |
73 MB |
1 |
sha512_100_bytes |
69.57 MB |
69.57 MB |
1 |
This comment was automatically generated by workflow using github-action-benchmark.
Description
Problem*
No issue just a minor optimization.
brillig_entry_points
will fulfill all the audit requirements post #9845, but I did notice that we perform unnecessary computation when we want to only get the set of Brillig entry points (as ininline_simple_functions
and once we bring back #9444). Getting the full mapping of Brillig entry points to the set of reachable functions requires doing a DFS on each entry point and also determining whether the entry point is a recursive function (SCCs on call graph). For large call graphs the cost of doing this unnecessarily could potentially grow significantly.Summary*
I made a method
get_brillig_entry_points
that simply returns the set of Brillig entry points. This method is then used internally byget_brillig_entry_points_with_recursive
andget_brillig_entry_points_with_reachability
(what used to beget_brillig_entry_points
).Additional Context
Does look like this change did in fact slightly reduce compilation times #9844 (review).
Documentation*
Check one:
PR Checklist*
cargo fmt
on default settings.