-
Notifications
You must be signed in to change notification settings - Fork 253
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
fix: Experiment with DecrementRc
#7629
Conversation
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: 19c19c0 | Previous: e3f8398 | Ratio |
---|---|---|---|
AztecProtocol_aztec-packages_noir-projects_noir-contracts |
99 s |
77 s |
1.29 |
This comment was automatically generated by workflow using github-action-benchmark.
CC: @TomAFrench
Opening this to review because I think this solves the original issue which #7297 patched. As far as I can see performance with this PR is similar to before it, although ref counts are slightly different depending on inliner settings. |
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.
LGTM just curious about the splitting up of reference_counts
test_programs/execution_success/reference_counts_inliner_0/src/main.nr
Outdated
Show resolved
Hide resolved
compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs
Outdated
Show resolved
Hide resolved
Co-authored-by: Maxim Vezenov <mvezenov@gmail.com>
Changes to Brillig bytecode sizes
🧾 Summary (10% most significant diffs)
Full diff report 👇
|
Automated pull of nightly from the [noir](https://github.com/noir-lang/noir) programming language, a dependency of Aztec. BEGIN_COMMIT_OVERRIDE chore: add `mapi`/`for_eachi` functions (noir-lang/noir#7705) fix: Experiment with `DecrementRc` (noir-lang/noir#7629) chore: delete honk programs from `test_programs` (noir-lang/noir#7727) chore(docs): Landing page jargonless intro (noir-lang/noir#7649) chore(docs): More acir docs (noir-lang/noir#7731) chore: add extra docs lint (noir-lang/noir#7742) fix: Fix stdout testing when running test programs (noir-lang/noir#7741) chore(ci): enforce rustdoc lints (noir-lang/noir#7738) chore: bump bb (noir-lang/noir#7726) fix: handle predicate value reduction in array get also for the databus (noir-lang/noir#7730) chore(refactor): Move resolved error structures out of acir crate (noir-lang/noir#7734) chore(ci): private-kernel-inner timeout bump (noir-lang/noir#7732) END_COMMIT_OVERRIDE --------- Co-authored-by: aakoshh <akosh@aztecprotocol.com> Co-authored-by: Akosh Farkash <aakoshh@gmail.com>
Description
Problem*
Resolves
Summary*
This is an experimental branch where I'm hoping to fix the underlying issues with our usage of
dec_rc
. Currently this PR:dec_rc value original_value
back to a single valueddec_rc value
dec_rc
on parameters to always be issued on the original load value rather than creating a new load. This should fix the original underflow issue we changed dec_rc for.reference_counts
into a separate test for each inliner setting: min, 0, and max. The inliner0
setting was actually seeing different optimization characteristics compared to the other two just due to optimization ordering. The behavior is still correct though.Additional Context
Documentation*
Check one:
PR Checklist*
cargo fmt
on default settings.