chore(perf): try using vec-collections's VecSet in AliasSet by asterite · Pull Request #7058 · noir-lang/noir

asterite · 2025-01-14T14:45:40Z

Description

Problem

Related to #7001

Summary

Alternative to #7057

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 · 2025-01-14T14:52:26Z

Execution Memory Report

Program	Peak Memory
keccak256	74.67M
workspace	123.72M
regression_4709	316.00M
ram_blowup_regression	512.61M
rollup-base-public	478.87M
rollup-base-private	325.48M
private-kernel-tail	180.47M
private-kernel-reset	245.16M
private-kernel-inner	208.55M

github-actions · 2025-01-14T14:52:35Z

Compilation Memory Report

Program	Peak Memory	%
keccak256	77.91M	-2%
workspace	123.59M	0%
regression_4709	424.09M	0%
ram_blowup_regression	1.58G	0%
rollup-base-public	3.50G	-25%
rollup-base-private	989.55M	100%
private-kernel-tail	207.18M	0%
private-kernel-reset	669.26M	0%
private-kernel-inner	294.40M	0%

github-actions · 2025-01-14T14:53:31Z

Execution Report

Program	Execution Time	%
sha256_regression	0.052s	-2%
regression_4709	0.001s	0%
ram_blowup_regression	0.606s	0%
rollup-root	0.105s	0%
rollup-merge	0.006s	0%
rollup-block-merge	0.105s	-1%
rollup-base-public	1.232s	0%
rollup-base-private	0.461s	1%
private-kernel-tail	0.019s	0%
private-kernel-reset	0.313s	0%
private-kernel-inner	0.068s	0%

github-actions · 2025-01-14T14:53:33Z

Compilation Report

Program	Compilation Time	%
sha256_regression	1.360s	10%
regression_4709	0.788s	0%
ram_blowup_regression	16.600s	-1%
rollup-root	3.430s	-2%
rollup-merge	1.980s	9%
rollup-block-merge	3.524s	2%
rollup-base-public	38.680s	-4%
rollup-base-private	14.540s	17%
private-kernel-tail	0.922s	-10%
private-kernel-reset	6.294s	-9%
private-kernel-inner	2.124s	4%

TomAFrench · 2025-01-14T14:55:53Z

Looks very similar in performance, up to you about which solution you prefer.

asterite · 2025-01-14T14:59:26Z

Yup, it seems it's exactly the same regarding memory :-)

Compilation times are different (on the left is the other PR):

But I guess those numbers vary more so we can't conclude one is better than the other.

I think I prefer this PR because there are less cases to handle. You?

asterite · 2025-01-14T15:04:44Z

Actually, I'll first ask Akosh to profile it locally to see if we get the same results as #7057 (comment)

TomAFrench · 2025-01-14T15:11:36Z

Those seem to all be within margin of error.

I think I prefer this PR because there are less cases to handle. You?

Yeah, I think as long as we're not feeling limited by this library then we can just stick with it. We know that we can easily chuck it if necessary.

aakoshh · 2025-01-14T15:20:38Z

This has virtually the same profile:

aakoshh

I would go with this one because d376513 had to repeat the entire match and I already used e.g. !self.is_unknown() instead of true for example. The earlier version with the 2 cases to consider for should_unify were easier to grok.

TomAFrench · 2025-01-14T15:52:31Z

@asterite Do you want to merge this as is or are there changes you want to make?

asterite · 2025-01-14T15:59:02Z

Yes, soon. Akosh asked me a few minutes to try to profile this together with his other PR.

TomAFrench · 2025-01-14T16:06:46Z

Will let you hit the merge button.

aakoshh · 2025-01-14T16:08:23Z

This is what it looks like with this merged into #7053

There is a strange plateuing of the memory, instead of the pronounced peak with the other variant.

But overall it looks similar:

…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)

…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)

…ir#7070) feat(LSP): code action to import trait in a method call (noir-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)

feat(LSP): code action to import trait in a method call (noir-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)

Automated pull of development from the [noir](https://github.com/noir-lang/noir) programming language, a dependency of Aztec. BEGIN_COMMIT_OVERRIDE chore: add end step for formatting workflow (noir-lang/noir#7070) feat(LSP): code action to import trait in a method call (noir-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) END_COMMIT_OVERRIDE --------- Co-authored-by: Tom French <tom@tomfren.ch> Co-authored-by: Tom French <15848336+TomAFrench@users.noreply.github.com> Co-authored-by: Jake Fecher <jfecher11@gmail.com>

asterite added 2 commits January 14, 2025 11:45

chore: try using vec-collections's VecSet in AliasSet

a979a80

clippy

88c27cb

Remove type annotation to avoid duplicating VecSet's type

0fa281a

asterite changed the title ~~chore: try using vec-collections's VecSet in AliasSet~~ chore(perf): try using vec-collections's VecSet in AliasSet Jan 14, 2025

aakoshh approved these changes Jan 14, 2025

View reviewed changes

TomAFrench marked this pull request as ready for review January 14, 2025 15:51

asterite added this pull request to the merge queue Jan 14, 2025

aakoshh mentioned this pull request Jan 14, 2025

fix: Reduce memory usage in mem2reg #7053

Merged

5 tasks

Merged via the queue into master with commit b6097a0 Jan 14, 2025
92 checks passed

asterite deleted the ab/mem2reg-alias-set-small-set branch January 14, 2025 16:27

AztecBot mentioned this pull request Jan 14, 2025

feat: Sync from noir AztecProtocol/aztec-packages#11196

Merged

AztecBot mentioned this pull request Jan 15, 2025

chore(master): Release 0.70.0 AztecProtocol/aztec-packages#11107

Merged

Conversation

asterite commented Jan 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Problem

Summary

Additional Context

Documentation

PR Checklist

Uh oh!

github-actions bot commented Jan 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Execution Memory Report

Uh oh!

github-actions bot commented Jan 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Compilation Memory Report

Uh oh!

github-actions bot commented Jan 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Execution Report

Uh oh!

github-actions bot commented Jan 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Compilation Report

Uh oh!

TomAFrench commented Jan 14, 2025

Uh oh!

asterite commented Jan 14, 2025

Uh oh!

asterite commented Jan 14, 2025

Uh oh!

TomAFrench commented Jan 14, 2025

Uh oh!

aakoshh commented Jan 14, 2025

Uh oh!

aakoshh left a comment

Choose a reason for hiding this comment

Uh oh!

TomAFrench commented Jan 14, 2025

Uh oh!

asterite commented Jan 14, 2025

Uh oh!

TomAFrench commented Jan 14, 2025

Uh oh!

aakoshh commented Jan 14, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

asterite commented Jan 14, 2025 •

edited

Loading

github-actions bot commented Jan 14, 2025 •

edited

Loading

github-actions bot commented Jan 14, 2025 •

edited

Loading

github-actions bot commented Jan 14, 2025 •

edited

Loading

github-actions bot commented Jan 14, 2025 •

edited

Loading