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.

Sign up for GitHub

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

Jump to bottom

chore: op queue cleanup #11925

Merged
merged 6 commits into from
Feb 12, 2025
Merged

chore: op queue cleanup #11925

merged 6 commits into from
Feb 12, 2025

Conversation

ledwards2225
Copy link
Contributor

@ledwards2225 ledwards2225 commented Feb 11, 2025 •
edited
Loading

Precursor cleanup of the ECCOpQueue class.

  • remove unused logic related to asynchronously updating the op queue
  • move logic for tracking ECCVM row usage into a sub-class EccvmRowTracker
  • general syntax cleanup

@ledwards2225 ledwards2225 marked this pull request as ready for review February 11, 2025 21:28
@ledwards2225 ledwards2225 self-assigned this Feb 11, 2025
lucasxia01
lucasxia01 approved these changes Feb 12, 2025
View reviewed changes
Copy link
Contributor

@lucasxia01 lucasxia01 left a comment

Choose a reason for hiding this comment

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

Looks good, thanks

barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/op_queue/ecc_op_queue.test.cpp
@@ -36,56 +36,3 @@ TEST(ECCOpQueueTest, InternalAccumulatorCorrectness)
op_queue.eq_and_reset();
EXPECT_TRUE(op_queue.get_accumulator().is_point_at_infinity());
}

TEST(ECCOpQueueTest, PrependAndSwapTests)
Copy link
Contributor

Choose a reason for hiding this comment

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

no replacement test?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

not for now. this is just functionality that we dont need and the nature of the op queue is about to change so even if we did need it, it would need to be totally revamped

barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/op_queue/eccvm_row_tracker.hpp
/**
* @brief Class for tracking the number of rows in the ECCVM circuit and the number of muls performed as the op queue is
* populated.
* @details This is to avoid expensive O(n) logic to compute the number of rows and muls during witness computation
Copy link
Contributor

Choose a reason for hiding this comment

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

was this really that expensive?

Copy link
Contributor

Choose a reason for hiding this comment

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

also I don't see where do we do O(n) stuff during witness computation

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 its not clear to me that there's any difference in doing this incrementally vs all at once but my goal here was just to isolate this existing logic and get it out of the EccOpQueue class. The comments are just duplicated from the original by @zac-williamson

@ledwards2225 ledwards2225 merged commit 082ed66 into master Feb 12, 2025
27 checks passed
@ledwards2225 ledwards2225 deleted the lde/op_queue_cleanup branch February 12, 2025 22:51
@AztecBot AztecBot mentioned this pull request Feb 12, 2025
Merged
sklppy88 pushed a commit that referenced this pull request Feb 13, 2025
@AztecBot
chore(master): Release 0.76.3 (#11924)
47fe362 
🤖 I have created a release *beep* *boop*
---


<details><summary>aztec-package: 0.76.3</summary>

##
[0.76.3](aztec-package-v0.76.2...aztec-package-v0.76.3)
(2025-02-12)


### Features

* Add undici
([#11818](#11818))
([8503c7a](8503c7a))
* Initial multi-proof test
([#11779](#11779))
([f54db75](f54db75))
</details>

<details><summary>barretenberg.js: 0.76.3</summary>

##
[0.76.3](barretenberg.js-v0.76.2...barretenberg.js-v0.76.3)
(2025-02-12)


### Features

* **perf:** Speed up construction of bbjs Frs & cache zero hashes in
ephemeral trees (redo)
([#11894](#11894))
([e093acf](e093acf))


### Bug Fixes

* Memory fragmentation fixes to cut UltraHonk memory usage by 26%
([#11895](#11895))
([b4e2264](b4e2264))
</details>

<details><summary>aztec-packages: 0.76.3</summary>

##
[0.76.3](aztec-packages-v0.76.2...aztec-packages-v0.76.3)
(2025-02-12)


### Features

* Add undici
([#11818](#11818))
([8503c7a](8503c7a))
* **avm:** Sequential lookup resolution
([#11769](#11769))
([3980f6c](3980f6c))
* Enable ws for reth on devnet
([#11922](#11922))
([7124664](7124664)),
closes
[#11921](#11921)
* Initial multi-proof test
([#11779](#11779))
([f54db75](f54db75))
* Native world state now supports checkpointing
([#11739](#11739))
([6464059](6464059))
* **perf:** Speed up construction of bbjs Frs & cache zero hashes in
ephemeral trees (redo)
([#11894](#11894))
([e093acf](e093acf))


### Bug Fixes

* Correctly configure batch queue
([#11934](#11934))
([4908df8](4908df8))
* De-dup pubkey conversion of cli-wallet param
([#11948](#11948))
([5529871](5529871))
* **docs:** Update the token bridge tutorial
([#11578](#11578))
([aaf42a7](aaf42a7))
* Don't restart kind control plane automatically
([#11923](#11923))
([c23c0f9](c23c0f9))
* Empty blocks can now be unwound
([#11920](#11920))
([fdc2042](fdc2042))
* Gcloud logs
([#11944](#11944))
([c53b1c5](c53b1c5)),
closes
[#11887](#11887)
* Lmdb cmake race condition
([#11959](#11959))
([031200d](031200d))
* Memory fragmentation fixes to cut UltraHonk memory usage by 26%
([#11895](#11895))
([b4e2264](b4e2264))
* Npm packages noir resolution
([#11946](#11946))
([d3e3f20](d3e3f20))
* Set resource limits on Loki pods
([#11940](#11940))
([6999982](6999982))


### Miscellaneous

* Only take FF (and not Flavor) in compute_logderivative_inverse
([#11938](#11938))
([bbbded3](bbbded3))
* Op queue cleanup
([#11925](#11925))
([082ed66](082ed66))
* Renaming `pxe_db.nr` as `capsules.nr`
([#11905](#11905))
([14d873c](14d873c))
* Replace relative paths to noir-protocol-circuits
([9ce401a](9ce401a))
* Use realistic size Client IVC proofs during simulations
([#11692](#11692))
([90b9fbf](90b9fbf))
* Use RelationChecker in relation correctness tests and add Translator
interleaving test
([#11878](#11878))
([ed215e8](ed215e8))
</details>

<details><summary>barretenberg: 0.76.3</summary>

##
[0.76.3](barretenberg-v0.76.2...barretenberg-v0.76.3)
(2025-02-12)


### Features

* **avm:** Sequential lookup resolution
([#11769](#11769))
([3980f6c](3980f6c))
* Native world state now supports checkpointing
([#11739](#11739))
([6464059](6464059))


### Bug Fixes

* Empty blocks can now be unwound
([#11920](#11920))
([fdc2042](fdc2042))
* Lmdb cmake race condition
([#11959](#11959))
([031200d](031200d))
* Memory fragmentation fixes to cut UltraHonk memory usage by 26%
([#11895](#11895))
([b4e2264](b4e2264))


### Miscellaneous

* Only take FF (and not Flavor) in compute_logderivative_inverse
([#11938](#11938))
([bbbded3](bbbded3))
* Op queue cleanup
([#11925](#11925))
([082ed66](082ed66))
* Use RelationChecker in relation correctness tests and add Translator
interleaving test
([#11878](#11878))
([ed215e8](ed215e8))
</details>

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).
AztecBot added a commit to AztecProtocol/barretenberg that referenced this pull request Feb 14, 2025
@AztecBot
chore(master): Release 0.76.3 (#11924)
ad2e48f 
🤖 I have created a release *beep* *boop*
---


<details><summary>aztec-package: 0.76.3</summary>

##
[0.76.3](AztecProtocol/aztec-packages@aztec-package-v0.76.2...aztec-package-v0.76.3)
(2025-02-12)


### Features

* Add undici
([#11818](AztecProtocol/aztec-packages#11818))
([8503c7a](AztecProtocol/aztec-packages@8503c7a))
* Initial multi-proof test
([#11779](AztecProtocol/aztec-packages#11779))
([f54db75](AztecProtocol/aztec-packages@f54db75))
</details>

<details><summary>barretenberg.js: 0.76.3</summary>

##
[0.76.3](AztecProtocol/aztec-packages@barretenberg.js-v0.76.2...barretenberg.js-v0.76.3)
(2025-02-12)


### Features

* **perf:** Speed up construction of bbjs Frs & cache zero hashes in
ephemeral trees (redo)
([#11894](AztecProtocol/aztec-packages#11894))
([e093acf](AztecProtocol/aztec-packages@e093acf))


### Bug Fixes

* Memory fragmentation fixes to cut UltraHonk memory usage by 26%
([#11895](AztecProtocol/aztec-packages#11895))
([b4e2264](AztecProtocol/aztec-packages@b4e2264))
</details>

<details><summary>aztec-packages: 0.76.3</summary>

##
[0.76.3](AztecProtocol/aztec-packages@aztec-packages-v0.76.2...aztec-packages-v0.76.3)
(2025-02-12)


### Features

* Add undici
([#11818](AztecProtocol/aztec-packages#11818))
([8503c7a](AztecProtocol/aztec-packages@8503c7a))
* **avm:** Sequential lookup resolution
([#11769](AztecProtocol/aztec-packages#11769))
([3980f6c](AztecProtocol/aztec-packages@3980f6c))
* Enable ws for reth on devnet
([#11922](AztecProtocol/aztec-packages#11922))
([7124664](AztecProtocol/aztec-packages@7124664)),
closes
[#11921](AztecProtocol/aztec-packages#11921)
* Initial multi-proof test
([#11779](AztecProtocol/aztec-packages#11779))
([f54db75](AztecProtocol/aztec-packages@f54db75))
* Native world state now supports checkpointing
([#11739](AztecProtocol/aztec-packages#11739))
([6464059](AztecProtocol/aztec-packages@6464059))
* **perf:** Speed up construction of bbjs Frs & cache zero hashes in
ephemeral trees (redo)
([#11894](AztecProtocol/aztec-packages#11894))
([e093acf](AztecProtocol/aztec-packages@e093acf))


### Bug Fixes

* Correctly configure batch queue
([#11934](AztecProtocol/aztec-packages#11934))
([4908df8](AztecProtocol/aztec-packages@4908df8))
* De-dup pubkey conversion of cli-wallet param
([#11948](AztecProtocol/aztec-packages#11948))
([5529871](AztecProtocol/aztec-packages@5529871))
* **docs:** Update the token bridge tutorial
([#11578](AztecProtocol/aztec-packages#11578))
([aaf42a7](AztecProtocol/aztec-packages@aaf42a7))
* Don't restart kind control plane automatically
([#11923](AztecProtocol/aztec-packages#11923))
([c23c0f9](AztecProtocol/aztec-packages@c23c0f9))
* Empty blocks can now be unwound
([#11920](AztecProtocol/aztec-packages#11920))
([fdc2042](AztecProtocol/aztec-packages@fdc2042))
* Gcloud logs
([#11944](AztecProtocol/aztec-packages#11944))
([c53b1c5](AztecProtocol/aztec-packages@c53b1c5)),
closes
[#11887](AztecProtocol/aztec-packages#11887)
* Lmdb cmake race condition
([#11959](AztecProtocol/aztec-packages#11959))
([031200d](AztecProtocol/aztec-packages@031200d))
* Memory fragmentation fixes to cut UltraHonk memory usage by 26%
([#11895](AztecProtocol/aztec-packages#11895))
([b4e2264](AztecProtocol/aztec-packages@b4e2264))
* Npm packages noir resolution
([#11946](AztecProtocol/aztec-packages#11946))
([d3e3f20](AztecProtocol/aztec-packages@d3e3f20))
* Set resource limits on Loki pods
([#11940](AztecProtocol/aztec-packages#11940))
([6999982](AztecProtocol/aztec-packages@6999982))


### Miscellaneous

* Only take FF (and not Flavor) in compute_logderivative_inverse
([#11938](AztecProtocol/aztec-packages#11938))
([bbbded3](AztecProtocol/aztec-packages@bbbded3))
* Op queue cleanup
([#11925](AztecProtocol/aztec-packages#11925))
([082ed66](AztecProtocol/aztec-packages@082ed66))
* Renaming `pxe_db.nr` as `capsules.nr`
([#11905](AztecProtocol/aztec-packages#11905))
([14d873c](AztecProtocol/aztec-packages@14d873c))
* Replace relative paths to noir-protocol-circuits
([9ce401a](AztecProtocol/aztec-packages@9ce401a))
* Use realistic size Client IVC proofs during simulations
([#11692](AztecProtocol/aztec-packages#11692))
([90b9fbf](AztecProtocol/aztec-packages@90b9fbf))
* Use RelationChecker in relation correctness tests and add Translator
interleaving test
([#11878](AztecProtocol/aztec-packages#11878))
([ed215e8](AztecProtocol/aztec-packages@ed215e8))
</details>

<details><summary>barretenberg: 0.76.3</summary>

##
[0.76.3](AztecProtocol/aztec-packages@barretenberg-v0.76.2...barretenberg-v0.76.3)
(2025-02-12)


### Features

* **avm:** Sequential lookup resolution
([#11769](AztecProtocol/aztec-packages#11769))
([3980f6c](AztecProtocol/aztec-packages@3980f6c))
* Native world state now supports checkpointing
([#11739](AztecProtocol/aztec-packages#11739))
([6464059](AztecProtocol/aztec-packages@6464059))


### Bug Fixes

* Empty blocks can now be unwound
([#11920](AztecProtocol/aztec-packages#11920))
([fdc2042](AztecProtocol/aztec-packages@fdc2042))
* Lmdb cmake race condition
([#11959](AztecProtocol/aztec-packages#11959))
([031200d](AztecProtocol/aztec-packages@031200d))
* Memory fragmentation fixes to cut UltraHonk memory usage by 26%
([#11895](AztecProtocol/aztec-packages#11895))
([b4e2264](AztecProtocol/aztec-packages@b4e2264))


### Miscellaneous

* Only take FF (and not Flavor) in compute_logderivative_inverse
([#11938](AztecProtocol/aztec-packages#11938))
([bbbded3](AztecProtocol/aztec-packages@bbbded3))
* Op queue cleanup
([#11925](AztecProtocol/aztec-packages#11925))
([082ed66](AztecProtocol/aztec-packages@082ed66))
* Use RelationChecker in relation correctness tests and add Translator
interleaving test
([#11878](AztecProtocol/aztec-packages#11878))
([ed215e8](AztecProtocol/aztec-packages@ed215e8))
</details>

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lucasxia01 lucasxia01 lucasxia01 approved these changes

Assignees

@ledwards2225 ledwards2225

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@ledwards2225 @lucasxia01