Skip to content

feat: Free instances and circuits earlier to reduce max memory usage#8118

Merged
lucasxia01 merged 6 commits intomasterfrom
lx/client-ivc-free-instance-and-circuits-earlier
Aug 22, 2024
Merged

feat: Free instances and circuits earlier to reduce max memory usage#8118
lucasxia01 merged 6 commits intomasterfrom
lx/client-ivc-free-instance-and-circuits-earlier

Conversation

@lucasxia01
Copy link
Contributor

@lucasxia01 lucasxia01 commented Aug 21, 2024

This PR reduces max memory by roughly a third by deleting a prover_instance member variable in client_ivc. Before, we were storing this prover_instance, which meant that when we accumulate a third circuit, we were storing three instances at the same time - the accumulator, the old instance, and the new instance. It also adjusts the test (ClientIVCTests.BasicThree) to only store the builder for when it's needed.

This is the original memory graph when running ClientIVCTests.BasicThree, which accumulates three circuits of size 2^17 (?).
Before

After freeing the Builder after constructing the instance from it, and after freeing the instance after folding it into the accumulator, our memory graph looks like.
After

You can see that the peak memory goes from 1216MB to 774MB, and that we're no longer storing 3 instances worth of data at one time.

@lucasxia01 lucasxia01 self-assigned this Aug 21, 2024
std::shared_ptr<VerifierInstance> verifier_accumulator;
// Note: We need to save the last instance that was folded in order to compute its verification key, this will not
// be needed in the real IVC as they are provided as inputs
std::shared_ptr<ProverInstance> prover_instance;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

main deletion here to free up memory

ivc.accumulate(kernel_circuit);

EXPECT_EQ(ivc.prover_instance->proving_key.log_circuit_size, 17);
EXPECT_EQ(ivc.fold_output.accumulator->proving_key.log_circuit_size, 17);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

core change here is that we don't store the prover_instance anymore, but we can just get the size from the accumulator instead.

// Go through each cycle
size_t cycle_index = 0;
for (auto& copy_cycle : wire_copy_cycles) {
for (const auto& copy_cycle : wire_copy_cycles) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

some changes Facundo suggested (not sure how impactful they are since the compiler is pretty smart usually)

@lucasxia01 lucasxia01 requested a review from maramihali August 22, 2024 15:19
@lucasxia01 lucasxia01 marked this pull request as ready for review August 22, 2024 15:19
@maramihali maramihali added the crypto cryptography label Aug 22, 2024
@lucasxia01 lucasxia01 merged commit 32a04c1 into master Aug 22, 2024
@lucasxia01 lucasxia01 deleted the lx/client-ivc-free-instance-and-circuits-earlier branch August 22, 2024 17:47
Copy link
Contributor

Nice!

lucasxia01 added a commit that referenced this pull request Aug 22, 2024
Continuation of
#8118.

In AztecIVC (or ClientIVC), when we have multiple instances, we create a
commitment key for each one. However, since each of these instances are
the same size, there's no need to create a new one for each one.

When we're constructing an instance beyond the first one, we can reuse
the same commitment key from the AztecIVC accumulator, which saves
~123MB of memory for 2^17 circuits, a reduction of 15.6%.
<img width="1045" alt="After"
src="https://github.com/user-attachments/assets/032cf442-5c68-4c23-b4d2-16ab8c6812b7">


After the change, we cut down max memory by 123MB.
<img width="969" alt="Before"
src="https://github.com/user-attachments/assets/8e374ab5-8a4b-4395-964e-35e49fe8920a">
AztecBot pushed a commit to AztecProtocol/barretenberg that referenced this pull request Aug 23, 2024
Continuation of
AztecProtocol/aztec-packages#8118.

In AztecIVC (or ClientIVC), when we have multiple instances, we create a
commitment key for each one. However, since each of these instances are
the same size, there's no need to create a new one for each one.

When we're constructing an instance beyond the first one, we can reuse
the same commitment key from the AztecIVC accumulator, which saves
~123MB of memory for 2^17 circuits, a reduction of 15.6%.
<img width="1045" alt="After"
src="https://github.com/user-attachments/assets/032cf442-5c68-4c23-b4d2-16ab8c6812b7">


After the change, we cut down max memory by 123MB.
<img width="969" alt="Before"
src="https://github.com/user-attachments/assets/8e374ab5-8a4b-4395-964e-35e49fe8920a">
spypsy pushed a commit that referenced this pull request Aug 23, 2024
🤖 I have created a release *beep* *boop*
---


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

##
[0.50.1](aztec-package-v0.50.0...aztec-package-v0.50.1)
(2024-08-23)


### Miscellaneous

* **aztec-package:** Synchronize aztec-packages versions
</details>

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

##
[0.50.1](barretenberg.js-v0.50.0...barretenberg.js-v0.50.1)
(2024-08-23)


### Miscellaneous

* **barretenberg.js:** Synchronize aztec-packages versions
</details>

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

##
[0.50.1](aztec-packages-v0.50.0...aztec-packages-v0.50.1)
(2024-08-23)


### Features

* Free instances and circuits earlier to reduce max memory usage
([#8118](#8118))
([32a04c1](32a04c1))
* Prepare protocol circuits for batch rollup
([#7727](#7727))
([a126e22](a126e22))
* Share the commitment key between instances to reduce mem
([#8154](#8154))
([c3dddf8](c3dddf8))


### Bug Fixes

* Cli-wallet manifest
([#8156](#8156))
([2ffcda3](2ffcda3))


### Miscellaneous

* Replace relative paths to noir-protocol-circuits
([5372ac4](5372ac4))
* Requiring only 1 sig from user
([#8146](#8146))
([f0b564b](f0b564b))
</details>

<details><summary>barretenberg: 0.50.1</summary>

##
[0.50.1](barretenberg-v0.50.0...barretenberg-v0.50.1)
(2024-08-23)


### Features

* Free instances and circuits earlier to reduce max memory usage
([#8118](#8118))
([32a04c1](32a04c1))
* Share the commitment key between instances to reduce mem
([#8154](#8154))
([c3dddf8](c3dddf8))
</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 Aug 24, 2024
🤖 I have created a release *beep* *boop*
---


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

##
[0.50.1](AztecProtocol/aztec-packages@aztec-package-v0.50.0...aztec-package-v0.50.1)
(2024-08-23)


### Miscellaneous

* **aztec-package:** Synchronize aztec-packages versions
</details>

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

##
[0.50.1](AztecProtocol/aztec-packages@barretenberg.js-v0.50.0...barretenberg.js-v0.50.1)
(2024-08-23)


### Miscellaneous

* **barretenberg.js:** Synchronize aztec-packages versions
</details>

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

##
[0.50.1](AztecProtocol/aztec-packages@aztec-packages-v0.50.0...aztec-packages-v0.50.1)
(2024-08-23)


### Features

* Free instances and circuits earlier to reduce max memory usage
([#8118](AztecProtocol/aztec-packages#8118))
([32a04c1](AztecProtocol/aztec-packages@32a04c1))
* Prepare protocol circuits for batch rollup
([#7727](AztecProtocol/aztec-packages#7727))
([a126e22](AztecProtocol/aztec-packages@a126e22))
* Share the commitment key between instances to reduce mem
([#8154](AztecProtocol/aztec-packages#8154))
([c3dddf8](AztecProtocol/aztec-packages@c3dddf8))


### Bug Fixes

* Cli-wallet manifest
([#8156](AztecProtocol/aztec-packages#8156))
([2ffcda3](AztecProtocol/aztec-packages@2ffcda3))


### Miscellaneous

* Replace relative paths to noir-protocol-circuits
([5372ac4](AztecProtocol/aztec-packages@5372ac4))
* Requiring only 1 sig from user
([#8146](AztecProtocol/aztec-packages#8146))
([f0b564b](AztecProtocol/aztec-packages@f0b564b))
</details>

<details><summary>barretenberg: 0.50.1</summary>

##
[0.50.1](AztecProtocol/aztec-packages@barretenberg-v0.50.0...barretenberg-v0.50.1)
(2024-08-23)


### Features

* Free instances and circuits earlier to reduce max memory usage
([#8118](AztecProtocol/aztec-packages#8118))
([32a04c1](AztecProtocol/aztec-packages@32a04c1))
* Share the commitment key between instances to reduce mem
([#8154](AztecProtocol/aztec-packages#8154))
([c3dddf8](AztecProtocol/aztec-packages@c3dddf8))
</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

Labels

crypto cryptography

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants