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(ci): Check various inliner aggressiveness setttings in Brillig reports #7049

Merged
merged 27 commits into from
Jan 14, 2025

Conversation

vezenovm
Copy link
Contributor

@vezenovm vezenovm commented Jan 13, 2025

Description

Problem*

Working towards slightly more comprehensive benchmarks. The github comments are definitely started to get a bit crowded as we try to benchmark more compiler settings.

Summary*

I changed the Brillig gate reports to accept an inliner aggressiveness setting. We now check compiling/executing with an inliner aggresiveness of i64::MAX (default), i64::MIN, and zero.

Additional Context

Testing in a follow-up #7052
Example output can be seen below as well:
Screenshot 2025-01-13 at 10 56 13 PM

Screenshot 2025-01-13 at 10 56 30 PM

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.

@vezenovm vezenovm marked this pull request as ready for review January 13, 2025 23:30
@vezenovm vezenovm requested a review from TomAFrench January 13, 2025 23:31
Copy link
Contributor

github-actions bot commented Jan 13, 2025

Compilation Memory Report

Program Peak Memory
keccak256 77.72M
workspace 123.82M
regression_4709 424.09M
ram_blowup_regression 1.58G
rollup-root 373.63M
rollup-merge 229.82M
rollup-block-root-single-tx 15.97G
rollup-block-root-empty 224.00M
rollup-block-root 15.97G
rollup-block-merge 373.62M
rollup-base-public 2.51G
rollup-base-private 901.11M
private-kernel-tail 207.28M
private-kernel-reset 669.36M
private-kernel-inner 294.50M

Copy link
Contributor

github-actions bot commented Jan 13, 2025

Execution Memory Report

Program Peak Memory
keccak256 74.67M
workspace 123.83M
regression_4709 316.00M
ram_blowup_regression 512.61M
rollup-root 231.12M
rollup-merge 205.85M
rollup-block-root 1.03G
rollup-block-merge 231.13M
rollup-base-public 478.88M
rollup-base-private 325.50M
private-kernel-tail 180.73M
private-kernel-reset 245.35M
private-kernel-inner 208.74M

Copy link
Contributor

github-actions bot commented Jan 14, 2025

Compilation Report

Program Compilation Time %
sha256_regression 1.070s -7%
regression_4709 0.803s -6%
ram_blowup_regression 16.300s -5%
rollup-root 3.332s -3%
rollup-merge 1.864s -4%
rollup-block-root-single-tx 147.000s -3%
rollup-block-root-empty 1.854s 0%
rollup-block-root 150.000s 2%
rollup-block-merge 3.598s 6%
rollup-base-public 31.940s 7%
rollup-base-private 10.640s -7%
private-kernel-tail 0.949s -2%
private-kernel-reset 6.224s 1%
private-kernel-inner 2.030s 1%

Copy link
Contributor

github-actions bot commented Jan 14, 2025

Execution Report

Program Execution Time %
sha256_regression 0.051s -2%
regression_4709 0.001s 0%
ram_blowup_regression 0.619s 1%
rollup-root 0.107s 2%
rollup-merge 0.006s -15%
rollup-block-root 38.300s 3%
rollup-block-merge 0.105s 0%
rollup-base-public 1.226s 0%
rollup-base-private 0.456s -1%
private-kernel-tail 0.019s 0%
private-kernel-reset 0.313s -1%
private-kernel-inner 0.068s -7%

Copy link
Member

@TomAFrench TomAFrench left a comment

Choose a reason for hiding this comment

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

LGTM

@vezenovm vezenovm added this pull request to the merge queue Jan 14, 2025
Merged via the queue into master with commit 51b7c07 Jan 14, 2025
98 checks passed
@vezenovm vezenovm deleted the mv/inliner-setting-brillig-reports branch January 14, 2025 20:50
AztecBot added a commit to AztecProtocol/aztec-packages that referenced this pull request Jan 14, 2025
…oir-lang/noir#7066)

feat!: Handle generic fields in `StructDefinition::fields` and move old functionality to `StructDefinition::fields_as_written` (noir-lang/noir#7067)
chore(ci): Check various inliner aggressiveness setttings in Brillig reports (noir-lang/noir#7049)
chore: reenable reports on rollup root circuits (noir-lang/noir#7061)
fix: don't always select trait impl when verifying trait constraints (noir-lang/noir#7041)
chore: mark some critical libraries as good again (noir-lang/noir#7065)
fix: Reduce memory usage in mem2reg (noir-lang/noir#7053)
feat: Allow associated types to be ellided from trait constraints (noir-lang/noir#7026)
chore(perf): try using vec-collections's VecSet in AliasSet (noir-lang/noir#7058)
chore: reduce number of iterations of `acvm::compiler::compile` (noir-lang/noir#7050)
chore: add `noir_check_shuffle` as a critical library (noir-lang/noir#7056)
chore: clippy warning fix (noir-lang/noir#7051)
chore(ci): Unify compilation/execution report jobs that take averages with single runs  (noir-lang/noir#7048)
fix(nargo_fmt): let doc comment could come after regular comment (noir-lang/noir#7046)
fix(nargo_fmt): don't consider identifiers the same if they are equal… (noir-lang/noir#7043)
feat: auto-import traits when suggesting trait methods (noir-lang/noir#7037)
feat: avoid inserting `inc_rc` instructions into ACIR (noir-lang/noir#7036)
fix(lsp): suggest all possible trait methods, but only visible ones (noir-lang/noir#7027)
chore: add more protocol circuits to reports (noir-lang/noir#6977)
feat: avoid generating a new witness when checking if linear expression is zero (noir-lang/noir#7031)
feat: skip codegen of zero iteration loops (noir-lang/noir#7030)
AztecBot added a commit to AztecProtocol/aztec-packages that referenced this pull request Jan 14, 2025
…ir#7066)

feat!: Handle generic fields in `StructDefinition::fields` and move old functionality to `StructDefinition::fields_as_written` (noir-lang/noir#7067)
chore(ci): Check various inliner aggressiveness setttings in Brillig reports (noir-lang/noir#7049)
chore: reenable reports on rollup root circuits (noir-lang/noir#7061)
fix: don't always select trait impl when verifying trait constraints (noir-lang/noir#7041)
chore: mark some critical libraries as good again (noir-lang/noir#7065)
fix: Reduce memory usage in mem2reg (noir-lang/noir#7053)
feat: Allow associated types to be ellided from trait constraints (noir-lang/noir#7026)
chore(perf): try using vec-collections's VecSet in AliasSet (noir-lang/noir#7058)
chore: reduce number of iterations of `acvm::compiler::compile` (noir-lang/noir#7050)
chore: add `noir_check_shuffle` as a critical library (noir-lang/noir#7056)
chore: clippy warning fix (noir-lang/noir#7051)
chore(ci): Unify compilation/execution report jobs that take averages with single runs  (noir-lang/noir#7048)
fix(nargo_fmt): let doc comment could come after regular comment (noir-lang/noir#7046)
fix(nargo_fmt): don't consider identifiers the same if they are equal… (noir-lang/noir#7043)
feat: auto-import traits when suggesting trait methods (noir-lang/noir#7037)
feat: avoid inserting `inc_rc` instructions into ACIR (noir-lang/noir#7036)
fix(lsp): suggest all possible trait methods, but only visible ones (noir-lang/noir#7027)
chore: add more protocol circuits to reports (noir-lang/noir#6977)
feat: avoid generating a new witness when checking if linear expression is zero (noir-lang/noir#7031)
feat: skip codegen of zero iteration loops (noir-lang/noir#7030)
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