Skip to content

Commit

Permalink
fix: avoid get row in databus (#5742)
Browse files Browse the repository at this point in the history
Removes use of get_row (which creates copies) from hot loop for
computing inverses for databus lookup relation.

Removes ~1.5 seconds:
`ClientIVCBench/Full/6      21580 ms        16468 ms            1`
  • Loading branch information
ledwards2225 committed Apr 12, 2024
1 parent 7d7a430 commit d67b6c8
Show file tree
Hide file tree
Showing 3 changed files with 18 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ void compute_logderivative_inverse(Polynomials& polynomials, auto& relation_para

auto& inverse_polynomial = lookup_relation.template get_inverse_polynomial(polynomials);
for (size_t i = 0; i < circuit_size; ++i) {
// TODO(https://github.com/AztecProtocol/barretenberg/issues/940): avoid get_row if possible.
auto row = polynomials.get_row(i);
bool has_inverse = lookup_relation.operation_exists_at_row(row);
if (!has_inverse) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,7 @@ template <class ProverInstances_> class ProtoGalaxyProver_ {
run_loop_in_parallel(instance_size, [&](size_t start_row, size_t end_row) {
auto thread_accumulator = FF(0);
for (size_t row = start_row; row < end_row; row++) {
// TODO(https://github.com/AztecProtocol/barretenberg/issues/940): avoid get_row if possible.
auto row_evaluations = instance_polynomials.get_row(row);
RelationEvaluations relation_evaluations;
Utils::zero_elements(relation_evaluations);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,8 @@ template <typename FF_> class DatabusLookupRelationImpl {
* @brief Construct the polynomial I whose components are the inverse of the product of the read and write terms
* @details If the denominators of log derivative lookup relation are read_term and write_term, then I_i =
* (read_term_i*write_term_i)^{-1}.
* @note Importantly, I_i = 0 for rows i at which there is no read or write.
* @note Importantly, I_i = 0 for rows i at which there is no read or write, so the cost of this method is
* proportional to the actual databus usage.
*
*/
template <size_t bus_idx, typename Polynomials>
Expand All @@ -186,11 +187,22 @@ template <typename FF_> class DatabusLookupRelationImpl {
const size_t circuit_size)
{
auto& inverse_polynomial = BusData<bus_idx, Polynomials>::inverses(polynomials);
// Compute the product of the read and write terms for each row
bool is_read = false;
bool nonzero_read_count = false;
for (size_t i = 0; i < circuit_size; ++i) {
auto row = polynomials.get_row(i);
// Determine if the present row contains a databus operation
auto& q_busread = polynomials.q_busread[i];
if constexpr (bus_idx == 0) { // calldata
is_read = q_busread == 1 && polynomials.q_l[i] == 1;
nonzero_read_count = polynomials.calldata_read_counts[i] > 0;
}
if constexpr (bus_idx == 1) { // return data
is_read = q_busread == 1 && polynomials.q_r[i] == 1;
nonzero_read_count = polynomials.return_data_read_counts[i] > 0;
}
// We only compute the inverse if this row contains a read gate or data that has been read
if (operation_exists_at_row<bus_idx>(row)) {
if (is_read || nonzero_read_count) {
auto row = polynomials.get_row(i); // Note: this is a copy. use sparingly!
inverse_polynomial[i] = compute_read_term<FF>(row, relation_parameters) *
compute_write_term<FF, bus_idx>(row, relation_parameters);
}
Expand Down

0 comments on commit d67b6c8

Please sign in to comment.