Skip to content

chore(ACIRgen): always compute array offset#11744

Closed
asterite wants to merge 1 commit intomasterfrom
ab/acir-compute_offset
Closed

chore(ACIRgen): always compute array offset#11744
asterite wants to merge 1 commit intomasterfrom
ab/acir-compute_offset

Conversation

@asterite
Copy link
Collaborator

@asterite asterite commented Mar 3, 2026

Description

Problem

No issue.

Summary

Revival of #10099 because I still think that PR is correct. I'll try it in Aztec-Packages again too. Maybe it wasn't working well because of other bugs we had back then, not sure.

Additional Context

User Documentation

Check one:

  • No user documentation needed.
  • Changes in docs/ included in this PR.
  • [For Experimental Features] Changes in docs/ 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
Copy link
Contributor

github-actions bot commented Mar 3, 2026

Changes to circuit sizes

Generated at commit: 1f8e27ae038f151fb6e76f06d7812a3f8ea9b6aa, compared to commit: 939f7c7e727fba20b7daf58856a6e3e6d4e34960

🧾 Summary (10% most significant diffs)

Program ACIR opcodes (+/-) % Circuit size (+/-) %
regression_struct_array_conditional 0 ➖ 0.00% -1 ✅ -0.03%
regression_11048 -2 ✅ -5.26% -2 ✅ -0.07%

Full diff report 👇
Program ACIR opcodes (+/-) % Circuit size (+/-) %
hashmap 29,356 (-1) -0.00% 90,309 (-1) -0.00%
regression_struct_array_conditional 68 (0) 0.00% 3,215 (-1) -0.03%
regression_11048 36 (-2) -5.26% 2,886 (-2) -0.07%

Copy link
Contributor

@github-actions github-actions bot left a 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 'Execution Time'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.20.

Benchmark suite Current: 3e5acf7 Previous: 939f7c7 Ratio
rollup-checkpoint-merge 0.003 s 0.002 s 1.50

This comment was automatically generated by workflow using github-action-benchmark.

CC: @TomAFrench

@jfecher
Copy link
Contributor

jfecher commented Mar 3, 2026

#10085 mentions we need to add test coverage due to the regression in aztec-packages. Did we ever add this? CI is green here but was before as well.

@asterite
Copy link
Collaborator Author

asterite commented Mar 3, 2026

Ah, no, because I think I never figured out what was the issue, or maybe I didn't investigate what the issue was, I can't remember.

@asterite
Copy link
Collaborator Author

asterite commented Mar 3, 2026

Well, it still fails: AztecProtocol/aztec-packages#21061

I still don't understand why, though, but for now I'll close this. I might also try to reproduce the issue locally.

@asterite asterite closed this Mar 3, 2026
@asterite asterite deleted the ab/acir-compute_offset branch March 3, 2026 15:51
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