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

feat: new op queue logic #11999

Merged
merged 17 commits into from
Feb 15, 2025
Merged

feat: new op queue logic #11999

merged 17 commits into from
Feb 15, 2025

Conversation

ledwards2225
Copy link
Contributor

@ledwards2225 ledwards2225 commented Feb 13, 2025

New class EccOpsTable to support the new concatenate-via-relations approach to the Merge protocol.

Background: The "Merge" protocol is essentially a protocol for establishing that a large table was constructed as the concatenation of smaller ones (subtables). In the original version of the protocol, the larger table was obtained by successively appending subtables (one from each circuit). The new version requires that subtables be PRE-pended. This results in a simpler protocol overall (and, importantly, one that's easier to make ZK) but its a bit more annoying from a data management standpoint since in general we don't want to pre-pend things in memory. This PR introduces a class EccOpsTable which stores individual subtables which can be virtually "prepended" to construct the entries of the corresponding aggregate table as needed.

@ledwards2225 ledwards2225 marked this pull request as ready for review February 14, 2025 22:12
@ledwards2225 ledwards2225 self-assigned this Feb 15, 2025
@ledwards2225 ledwards2225 requested a review from ludamad February 15, 2025 00:06
: subtable(sub)
, index(idx)
, subtable_size(sub ? sub->size() : 0)
{}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nit: could get away without storing size

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah I did this to save on the size() call every time in a hot loop but maybe thats overkill. I would def prefer just calling size()

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yeah so what's going to happen:

  • we are reading a pointer, offsetting it (basically free), then reading a field
    vs
  • we are reading a field
    the pointer we are dereferencing will be in cache, so it's a trivial difference

Copy link
Collaborator

Choose a reason for hiding this comment

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

We are also returning a rather big struct, which, fair if we are reading one field from it might be a hot loop, but if we are doing anything with a field the loop overhead is not that big. Comes down to how many times we will be iterating during a proving lifetime, I guess. Could be worth the cycles potentially

@ledwards2225 ledwards2225 merged commit de7882c into master Feb 15, 2025
13 checks passed
@ledwards2225 ledwards2225 deleted the lde/new_op_queue branch February 15, 2025 19:53
TomAFrench added a commit that referenced this pull request Feb 17, 2025
* master: (207 commits)
  chore(docs): acir formal verification final report (#12040)
  feat(avm): packed field in bytecode decomposition (#12015)
  feat: Contract updates (#11514)
  feat!: enforce fees (#11480)
  chore: redo typo PR by maximevtush (#12033)
  git subrepo push --branch=master noir-projects/aztec-nr
  git_subrepo.sh: Fix parent in .gitrepo file. [skip ci]
  chore: replace relative paths to noir-protocol-circuits
  git subrepo push --branch=master barretenberg
  fix: unexposing test fr from vkey struct ts (#12028)
  chore: structured polys in Translator (#12003)
  fix(ci3): fix ./bootstrap.sh fast in noir-projects (#12026)
  feat: new op queue logic (#11999)
  git subrepo push --branch=master noir-projects/aztec-nr
  git_subrepo.sh: Fix parent in .gitrepo file. [skip ci]
  chore: replace relative paths to noir-protocol-circuits
  git subrepo push --branch=master barretenberg
  chore: skip flakey p2p (#12021)
  chore(ci3): label handling (#12020)
  feat: Sync from noir (#12002)
  ...
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