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

feat: Parallelise kernel and function circuit construction in client IVC #4841

Merged
merged 4 commits into from
Feb 29, 2024
+86 −22
Merged

feat: Parallelise kernel and function circuit construction in client IVC #4841

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
de32727
Parallelise function and kernel construction
Rumata888 Feb 27, 2024
8073925
Removed extra lines
Rumata888 Feb 28, 2024
e253a77
Merge branch 'master' into my domain
Rumata888 Feb 29, 2024
5949522
here we go again
Rumata888 Feb 29, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
61 changes: 48 additions & 13 deletions barretenberg/cpp/src/barretenberg/benchmark/client_ivc_bench/client_ivc.bench.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,24 +46,59 @@ class ClientIVCBench : public benchmark::Fixture {
GoblinMockCircuits::construct_mock_function_circuit(function_circuit);
ivc.initialize(function_circuit);

// Accumulate kernel circuit (first kernel mocked as simple circuit since no folding proofs yet)
Copy link
Contributor

Choose a reason for hiding this comment

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

You've done the multithreading in a way such that only 5 out of the 6 iterations are being parallelized. Why not start the parallelization with the very first iteration?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because we technically can't. The kernel is supposed to work with the previous proof, so we can only construct the next function circuit, while we are constructing the kernel, not the previous one. If we did, it would be cheating

Copy link
Contributor

Choose a reason for hiding this comment

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

got it, makes sense

Builder kernel_circuit{ ivc.goblin.op_queue };
GoblinMockCircuits::construct_mock_function_circuit(kernel_circuit);
auto kernel_fold_proof = ivc.accumulate(kernel_circuit);

auto NUM_CIRCUITS = static_cast<size_t>(state.range(0));
NUM_CIRCUITS -= 1; // Subtract one to account for the "initialization" round above
for (size_t circuit_idx = 0; circuit_idx < NUM_CIRCUITS; ++circuit_idx) {

// Accumulate function circuit
Builder function_circuit{ ivc.goblin.op_queue };
GoblinMockCircuits::construct_mock_function_circuit(function_circuit);
auto function_fold_proof = ivc.accumulate(function_circuit);
ClientIVC::FoldProof function_fold_proof;
ClientIVC::FoldProof kernel_fold_proof;
for (size_t circuit_idx = 0; circuit_idx < NUM_CIRCUITS; ++circuit_idx) {
std::array<Builder, 2> kernel_and_function_builders;
parallel_for(2, [&](size_t workload_index) {
if (workload_index == 0) {
// Initialize with current op_queue
kernel_and_function_builders[workload_index] = Builder(ivc.goblin.op_queue);
// Compute kernel circuit
if (circuit_idx == 0) {
// First kernel is not really a kernel
GoblinMockCircuits::construct_mock_function_circuit(
kernel_and_function_builders[workload_index]);
} else {
// Actually kernel for next
GoblinMockCircuits::construct_mock_folding_kernel(
kernel_and_function_builders[workload_index], function_fold_proof, kernel_fold_proof);
}
} else {
// Construct function circuit in parallel
// Initialize without op_queue
kernel_and_function_builders[workload_index] = Builder();
GoblinMockCircuits::construct_mock_function_circuit(kernel_and_function_builders[workload_index]);
}
});
// Accumulate kernel
kernel_fold_proof = ivc.accumulate(kernel_and_function_builders[0]);

// Prepend op_queue to function circuit
kernel_and_function_builders[1].op_queue->prepend_previous_queue(*ivc.goblin.op_queue);
Copy link
Contributor

Choose a reason for hiding this comment

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

this prepending stuff to the op_queue seems to be newly introduced. Why the change?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because there was no functionality allowing such updates to the queue and it is needed for correctness (we need to prepend previous queue so that the one being accumulated is correct, but we don't care about the previous queue during function circuit construction)


// Accumulate circuit
function_fold_proof = ivc.accumulate(kernel_and_function_builders[1]);

// Retrieve op_queue from function circuit
std::swap(*ivc.goblin.op_queue, *kernel_and_function_builders[1].op_queue);
}

// Perform last kernel accumulation
if (NUM_CIRCUITS == 0) {
// If we only do 1 fold in total, use mock function circuit
Builder last_kernel_circuit{ ivc.goblin.op_queue };
GoblinMockCircuits::construct_mock_function_circuit(last_kernel_circuit);
auto kernel_fold_proof = ivc.accumulate(last_kernel_circuit);
} else {
// Accumulate kernel circuit
Builder kernel_circuit{ ivc.goblin.op_queue };
GoblinMockCircuits::construct_mock_folding_kernel(kernel_circuit, function_fold_proof, kernel_fold_proof);
auto kernel_fold_proof = ivc.accumulate(kernel_circuit);
Builder last_kernel_circuit{ ivc.goblin.op_queue };
GoblinMockCircuits::construct_mock_folding_kernel(
last_kernel_circuit, function_fold_proof, kernel_fold_proof);
auto kernel_fold_proof = ivc.accumulate(last_kernel_circuit);
}
}
};
Expand Down
4 changes: 2 additions & 2 deletions barretenberg/cpp/src/barretenberg/goblin/goblin.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ class Goblin {
auto ultra_proof = prover.construct_proof();

// Construct and store the merge proof to be recursively verified on the next call to accumulate
MergeProver merge_prover{ op_queue };
MergeProver merge_prover{ circuit_builder.op_queue };
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We need the prover to be dependent on the current circuit builder instead of taking information from the global op_queue.

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 a bug?

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 really. Since we only did circuits sequentially, we didn't care about this, since both pointers pointed to the same object.

Copy link
Contributor

Choose a reason for hiding this comment

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

ah I see

merge_proof = merge_prover.construct_proof();

if (!merge_proof_exists) {
Expand Down Expand Up @@ -137,7 +137,7 @@ class Goblin {
}

// Construct and store the merge proof to be recursively verified on the next call to accumulate
MergeProver merge_prover{ op_queue };
MergeProver merge_prover{ circuit_builder.op_queue };
merge_proof = merge_prover.construct_proof();

if (!merge_proof_exists) {
Expand Down
7 changes: 7 additions & 0 deletions barretenberg/cpp/src/barretenberg/proof_system/op_queue/ecc_op_queue.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,13 @@ class ECCOpQueue {
ultra_ops_commitments = previous.ultra_ops_commitments;
previous_ultra_ops_commitments = previous.previous_ultra_ops_commitments;
}
/**
* @brief Prepend the information from the previous queue (used before accumulation/merge proof to be able to run
* circuit construction separately)
*
* @param previous_ptr
*/
void prepend_previous_queue(const ECCOpQueue* previous_ptr) { prepend_previous_queue(*previous_ptr); }

/**
* @brief Enable using std::swap on queues
Expand Down
Loading