Skip to content

Commit

Permalink
chore(avm): full row cleanup (#11767)
Browse files Browse the repository at this point in the history
Please read [contributing guidelines](CONTRIBUTING.md) and remove this line.
  • Loading branch information
fcarreiro authored Feb 6, 2025
1 parent ae79400 commit 6145cd0
Show file tree
Hide file tree
Showing 11 changed files with 5 additions and 100 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -58,14 +58,14 @@ AvmCircuitBuilder::ProverPolynomials AvmCircuitBuilder::compute_polynomials() co
// An array which stores for each column of the trace the smallest size of the
// truncated column containing all non-zero elements.
// It is used to allocate the polynomials without memory overhead for the tail of zeros.
std::array<size_t, Row::SIZE> col_nonzero_size{};
std::array<size_t, NUM_COLUMNS_WITH_SHIFTS> col_nonzero_size{};

// Computation of size of columns.
// Non-parallel version takes 0.5 second for a trace size of 200k rows.
// A parallel version might be considered in the future.
for (size_t i = 0; i < num_rows; i++) {
const auto& row = rows[i];
for (size_t col = 0; col < Row::SIZE; col++) {
for (size_t col = 0; col < col_nonzero_size.size(); col++) {
if (!row.get_column(static_cast<ColumnAndShifts>(col)).is_zero()) {
col_nonzero_size[col] = i + 1;
}
Expand Down
16 changes: 0 additions & 16 deletions barretenberg/cpp/src/barretenberg/vm/avm/generated/full_row.cpp

This file was deleted.

Original file line number Diff line number Diff line change
@@ -1,11 +1,6 @@
// AUTOGENERATED FILE
#pragma once

#include <sstream>
#include <string>
#include <vector>

#include "barretenberg/common/ref_vector.hpp"
#include "columns.hpp"

namespace bb::avm {
Expand All @@ -15,8 +10,6 @@ template <typename FF_> struct AvmFullRow {

FF AVM_ALL_ENTITIES;

static constexpr size_t SIZE = 764;

// Risky but oh so efficient.
FF& get_column(ColumnAndShifts col)
{
Expand All @@ -31,8 +24,6 @@ template <typename FF_> struct AvmFullRow {
}
};

template <typename FF> std::ostream& operator<<(std::ostream& os, AvmFullRow<FF> const& row);

} // namespace bb::avm

namespace bb {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ std::unordered_map</*relation*/ std::string, /*degrees*/ std::string> get_relati
void show_trace_info(const auto& trace)
{
vinfo("Built trace size: ", trace.size(), " (next power: 2^", std::bit_width(trace.size()), ")");
vinfo("Number of columns: ", trace.front().SIZE);
vinfo("Number of columns: ", avm::NUM_COLUMNS_WITH_SHIFTS);
vinfo("Relation degrees: ", []() {
std::string result;
for (const auto& [key, value] : sorted_entries(get_relations_degrees())) {
Expand Down
15 changes: 0 additions & 15 deletions barretenberg/cpp/src/barretenberg/vm2/generated/full_row.cpp

This file was deleted.

9 changes: 0 additions & 9 deletions barretenberg/cpp/src/barretenberg/vm2/generated/full_row.hpp
Original file line number Diff line number Diff line change
@@ -1,11 +1,6 @@
// AUTOGENERATED FILE
#pragma once

#include <sstream>
#include <string>
#include <vector>

#include "barretenberg/common/ref_vector.hpp"
#include "columns.hpp"

namespace bb::avm2 {
Expand All @@ -15,8 +10,6 @@ template <typename FF_> struct AvmFullRow {

FF AVM2_ALL_ENTITIES;

static constexpr size_t SIZE = 398;

// Risky but oh so efficient.
FF& get_column(ColumnAndShifts col)
{
Expand All @@ -31,8 +24,6 @@ template <typename FF_> struct AvmFullRow {
}
};

template <typename FF> std::ostream& operator<<(std::ostream& os, AvmFullRow<FF> const& row);

} // namespace bb::avm2

namespace bb {
Expand Down
21 changes: 0 additions & 21 deletions bb-pilcom/bb-pil-backend/src/circuit_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ pub trait CircuitBuilder {
fn create_circuit_builder_cpp(&mut self, name: &str, all_cols_without_inverses: &[String]);

fn create_full_row_hpp(&mut self, name: &str, all_cols: &[String]);
fn create_full_row_cpp(&mut self, name: &str, all_cols: &[String]);
}

impl CircuitBuilder for BBFiles {
Expand Down Expand Up @@ -71,24 +70,4 @@ impl CircuitBuilder for BBFiles {

self.write_file(None, "full_row.hpp", &hpp);
}

fn create_full_row_cpp(&mut self, name: &str, all_cols: &[String]) {
let mut handlebars = Handlebars::new();

let data = &json!({
"name": name,
"all_cols": all_cols,
});

handlebars
.register_template_string(
"full_row.cpp",
std::str::from_utf8(include_bytes!("../templates/full_row.cpp.hbs")).unwrap(),
)
.unwrap();

let cpp = handlebars.render("full_row.cpp", data).unwrap();

self.write_file(None, "full_row.cpp", &cpp);
}
}
1 change: 0 additions & 1 deletion bb-pilcom/bb-pil-backend/src/vm_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,6 @@ pub fn analyzed_to_cpp<F: FieldElement>(

// ----------------------- Create the full row files -----------------------
bb_files.create_full_row_hpp(vm_name, &all_cols);
bb_files.create_full_row_cpp(vm_name, &all_cols);

// ----------------------- Create the flavor files -----------------------
bb_files.create_flavor_hpp(
Expand Down
4 changes: 2 additions & 2 deletions bb-pilcom/bb-pil-backend/templates/circuit_builder.cpp.hbs
Original file line number Diff line number Diff line change
Expand Up @@ -58,14 +58,14 @@ AvmCircuitBuilder::ProverPolynomials AvmCircuitBuilder::compute_polynomials() co
// An array which stores for each column of the trace the smallest size of the
// truncated column containing all non-zero elements.
// It is used to allocate the polynomials without memory overhead for the tail of zeros.
std::array<size_t, Row::SIZE> col_nonzero_size{};
std::array<size_t, NUM_COLUMNS_WITH_SHIFTS> col_nonzero_size{};

// Computation of size of columns.
// Non-parallel version takes 0.5 second for a trace size of 200k rows.
// A parallel version might be considered in the future.
for (size_t i = 0; i < num_rows; i++) {
const auto& row = rows[i];
for (size_t col = 0; col < Row::SIZE; col++) {
for (size_t col = 0; col < col_nonzero_size.size(); col++) {
if (!row.get_column(static_cast<ColumnAndShifts>(col)).is_zero()) {
col_nonzero_size[col] = i + 1;
}
Expand Down
15 changes: 0 additions & 15 deletions bb-pilcom/bb-pil-backend/templates/full_row.cpp.hbs

This file was deleted.

9 changes: 0 additions & 9 deletions bb-pilcom/bb-pil-backend/templates/full_row.hpp.hbs
Original file line number Diff line number Diff line change
@@ -1,11 +1,6 @@
// AUTOGENERATED FILE
#pragma once

#include <string>
#include <sstream>
#include <vector>

#include "barretenberg/common/ref_vector.hpp"
#include "columns.hpp"

namespace bb::{{snakeCase name}} {
Expand All @@ -16,8 +11,6 @@ struct AvmFullRow {

FF {{shoutySnakeCase name}}_ALL_ENTITIES;

static constexpr size_t SIZE = {{len all_cols}};

// Risky but oh so efficient.
FF& get_column(ColumnAndShifts col)
{
Expand All @@ -32,8 +25,6 @@ struct AvmFullRow {
}
};

template <typename FF> std::ostream& operator<<(std::ostream& os, AvmFullRow<FF> const& row);

} // namespace bb::{{snakeCase name}}

namespace bb {
Expand Down

1 comment on commit 6145cd0

@AztecBot
Copy link
Collaborator

Choose a reason for hiding this comment

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

⚠️ Performance Alert ⚠️

Possible performance regression was detected for benchmark 'C++ Benchmark'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.05.

Benchmark suite Current: 6145cd0 Previous: b003567 Ratio
nativeconstruct_proof_ultrahonk_power_of_2/20 4490.174085000007 ms/iter 4143.899132999991 ms/iter 1.08
Goblin::merge(t) 146876412 ns/iter 135142599 ns/iter 1.09

This comment was automatically generated by workflow using github-action-benchmark.

CC: @ludamad @codygunton

Please sign in to comment.