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

shared_ptr is overused in BB #4885

Open
fcarreiro opened this issue Mar 1, 2024 · 0 comments
Open

shared_ptr is overused in BB #4885

fcarreiro opened this issue Mar 1, 2024 · 0 comments
Assignees
Labels
C-barretenberg Component: barretenberg cryptography library

Comments

@fcarreiro
Copy link
Contributor

https://aztecprotocol.slack.com/archives/C01CGB1KUN4/p1709300900813939

Refactor to use unique_ptr and T& / T* when possible.

@fcarreiro fcarreiro added the C-barretenberg Component: barretenberg cryptography library label Mar 1, 2024
@github-project-automation github-project-automation bot moved this to Todo in A3 Mar 1, 2024
@ludamad ludamad changed the title shared_ptr is abused in BB shared_ptr is overused in BB Mar 4, 2024
@lucasxia01 lucasxia01 self-assigned this Mar 22, 2024
lucasxia01 added a commit that referenced this issue Apr 1, 2024
…5407)

Partially addresses
#4885.

Removes shared_ptr usage for ultra/goblin_ultra proving key.

Shouldn't affect the benchmarks much, but for some reason it seems to
have decreased slightly.

```
--------------------------------------------------------------------------------
Benchmark                      Time             CPU   Iterations UserCounters...
--------------------------------------------------------------------------------
ClientIVCBench/Full/6      22099 ms        17172 ms            1 Decider::construct_proof=1 Decider::construct_proof(t)=754.682M ECCVMComposer::compute_commitment_key=1 ECCVMComposer::compute_commitment_key(t)=3.65834M ECCVMComposer::compute_witness=1 ECCVMComposer::compute_witness(t)=130.21M ECCVMComposer::create_prover=1 ECCVMComposer::create_prover(t)=149.963M ECCVMComposer::create_proving_key=1 ECCVMComposer::create_proving_key(t)=15.8747M ECCVMProver::construct_proof=1 ECCVMProver::construct_proof(t)=1.75623G Goblin::merge=11 Goblin::merge(t)=142.793M GoblinTranslatorCircuitBuilder::constructor=1 GoblinTranslatorCircuitBuilder::constructor(t)=58.4782M GoblinTranslatorProver=1 GoblinTranslatorProver(t)=144.954M GoblinTranslatorProver::construct_proof=1 GoblinTranslatorProver::construct_proof(t)=962.221M ProtoGalaxyProver_::accumulator_update_round=10 ProtoGalaxyProver_::accumulator_update_round(t)=294.093M ProtoGalaxyProver_::combiner_quotient_round=10 ProtoGalaxyProver_::combiner_quotient_round(t)=5.93984G ProtoGalaxyProver_::perturbator_round=10 ProtoGalaxyProver_::perturbator_round(t)=1.29776G ProtoGalaxyProver_::preparation_round=10 ProtoGalaxyProver_::preparation_round(t)=4.1465G ProtogalaxyProver::fold_instances=10 ProtogalaxyProver::fold_instances(t)=11.6782G ProverInstance(Circuit&)=11 ProverInstance(Circuit&)(t)=1.97873G batch_mul_with_endomorphism=30 batch_mul_with_endomorphism(t)=565.226M commit=426 commit(t)=4.01591G compute_combiner=10 compute_combiner(t)=5.93777G compute_perturbator=9 compute_perturbator(t)=1.29745G compute_univariate=48 compute_univariate(t)=1.42388G construct_circuits=6 construct_circuits(t)=4.44634G
Benchmarking lock deleted.
client_ivc_bench.json                                                            100% 3999   115.8KB/s   00:00    
function                                        ms     % sum
construct_circuits(t)                         4446    20.33%
ProverInstance(Circuit&)(t)                   1979     9.05%
ProtogalaxyProver::fold_instances(t)         11678    53.40%
Decider::construct_proof(t)                    755     3.45%
ECCVMComposer::create_prover(t)                150     0.69%
ECCVMProver::construct_proof(t)               1756     8.03%
GoblinTranslatorProver::construct_proof(t)     962     4.40%
Goblin::merge(t)                               143     0.65%

Total time accounted for: 21869ms/22099ms = 98.96%

Major contributors:
function                                        ms    % sum
commit(t)                                     4016   18.36%
compute_combiner(t)                           5938   27.15%
compute_perturbator(t)                        1297    5.93%
compute_univariate(t)                         1424    6.51%

Breakdown of ProtogalaxyProver::fold_instances:
ProtoGalaxyProver_::preparation_round(t)           4147    35.51%
ProtoGalaxyProver_::perturbator_round(t)           1298    11.11%
ProtoGalaxyProver_::combiner_quotient_round(t)     5940    50.86%
ProtoGalaxyProver_::accumulator_update_round(t)     294     2.52%
```

Please read [contributing guidelines](CONTRIBUTING.md) and remove this
line.
AztecBot pushed a commit to AztecProtocol/barretenberg that referenced this issue Apr 2, 2024
…(#5407)

Partially addresses
AztecProtocol/aztec-packages#4885.

Removes shared_ptr usage for ultra/goblin_ultra proving key.

Shouldn't affect the benchmarks much, but for some reason it seems to
have decreased slightly.

```
--------------------------------------------------------------------------------
Benchmark                      Time             CPU   Iterations UserCounters...
--------------------------------------------------------------------------------
ClientIVCBench/Full/6      22099 ms        17172 ms            1 Decider::construct_proof=1 Decider::construct_proof(t)=754.682M ECCVMComposer::compute_commitment_key=1 ECCVMComposer::compute_commitment_key(t)=3.65834M ECCVMComposer::compute_witness=1 ECCVMComposer::compute_witness(t)=130.21M ECCVMComposer::create_prover=1 ECCVMComposer::create_prover(t)=149.963M ECCVMComposer::create_proving_key=1 ECCVMComposer::create_proving_key(t)=15.8747M ECCVMProver::construct_proof=1 ECCVMProver::construct_proof(t)=1.75623G Goblin::merge=11 Goblin::merge(t)=142.793M GoblinTranslatorCircuitBuilder::constructor=1 GoblinTranslatorCircuitBuilder::constructor(t)=58.4782M GoblinTranslatorProver=1 GoblinTranslatorProver(t)=144.954M GoblinTranslatorProver::construct_proof=1 GoblinTranslatorProver::construct_proof(t)=962.221M ProtoGalaxyProver_::accumulator_update_round=10 ProtoGalaxyProver_::accumulator_update_round(t)=294.093M ProtoGalaxyProver_::combiner_quotient_round=10 ProtoGalaxyProver_::combiner_quotient_round(t)=5.93984G ProtoGalaxyProver_::perturbator_round=10 ProtoGalaxyProver_::perturbator_round(t)=1.29776G ProtoGalaxyProver_::preparation_round=10 ProtoGalaxyProver_::preparation_round(t)=4.1465G ProtogalaxyProver::fold_instances=10 ProtogalaxyProver::fold_instances(t)=11.6782G ProverInstance(Circuit&)=11 ProverInstance(Circuit&)(t)=1.97873G batch_mul_with_endomorphism=30 batch_mul_with_endomorphism(t)=565.226M commit=426 commit(t)=4.01591G compute_combiner=10 compute_combiner(t)=5.93777G compute_perturbator=9 compute_perturbator(t)=1.29745G compute_univariate=48 compute_univariate(t)=1.42388G construct_circuits=6 construct_circuits(t)=4.44634G
Benchmarking lock deleted.
client_ivc_bench.json                                                            100% 3999   115.8KB/s   00:00    
function                                        ms     % sum
construct_circuits(t)                         4446    20.33%
ProverInstance(Circuit&)(t)                   1979     9.05%
ProtogalaxyProver::fold_instances(t)         11678    53.40%
Decider::construct_proof(t)                    755     3.45%
ECCVMComposer::create_prover(t)                150     0.69%
ECCVMProver::construct_proof(t)               1756     8.03%
GoblinTranslatorProver::construct_proof(t)     962     4.40%
Goblin::merge(t)                               143     0.65%

Total time accounted for: 21869ms/22099ms = 98.96%

Major contributors:
function                                        ms    % sum
commit(t)                                     4016   18.36%
compute_combiner(t)                           5938   27.15%
compute_perturbator(t)                        1297    5.93%
compute_univariate(t)                         1424    6.51%

Breakdown of ProtogalaxyProver::fold_instances:
ProtoGalaxyProver_::preparation_round(t)           4147    35.51%
ProtoGalaxyProver_::perturbator_round(t)           1298    11.11%
ProtoGalaxyProver_::combiner_quotient_round(t)     5940    50.86%
ProtoGalaxyProver_::accumulator_update_round(t)     294     2.52%
```

Please read [contributing guidelines](CONTRIBUTING.md) and remove this
line.
@fcarreiro fcarreiro removed their assignment Oct 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-barretenberg Component: barretenberg cryptography library
Projects
Status: Todo
Development

No branches or pull requests

3 participants