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

feat: Add gate profiler for noir circuits #7004

Merged
merged 52 commits into from
Jun 13, 2024
Merged
Show file tree
Hide file tree
Changes from 44 commits
Commits
Show all changes
52 commits
Select commit Hold shift + click to select a range
f6d65af
initial version of reporter of gates
sirasistant Jun 6, 2024
14aa792
amortising table gates over all opcodes which use the table... ish
iAmMichaelConnor Jun 7, 2024
455dcbb
feat: flamegraph generator
sirasistant Jun 7, 2024
1b7753e
Merge branch 'arv/flamegraph_acir' of github.com:AztecProtocol/aztec-…
sirasistant Jun 7, 2024
7a20a94
Revert "amortising table gates over all opcodes which use the table..…
sirasistant Jun 7, 2024
319c980
it's workiiiiiiiiing
sirasistant Jun 7, 2024
64453b8
Reapply "amortising table gates over all opcodes which use the table.…
iAmMichaelConnor Jun 7, 2024
fc7b22a
better spreading
iAmMichaelConnor Jun 7, 2024
5da2664
try again... it's over-computing gates
iAmMichaelConnor Jun 7, 2024
159605d
and again...
iAmMichaelConnor Jun 7, 2024
1ba3b38
flamegraph options
sirasistant Jun 10, 2024
db6db28
Merge branch 'arv/flamegraph_acir' of github.com:AztecProtocol/aztec-…
sirasistant Jun 10, 2024
0b3d59e
remove simple test
sirasistant Jun 10, 2024
987c683
add smol script for extract a contract function as a regular artifact
sirasistant Jun 10, 2024
15f2c2b
refactor create flamegraph cmd
sirasistant Jun 10, 2024
e98efed
remove some extraneous boilerplate
sirasistant Jun 10, 2024
12b9c17
rename
sirasistant Jun 11, 2024
75885c8
update help
sirasistant Jun 11, 2024
2456442
simplify bb changes to gate counter
sirasistant Jun 11, 2024
c6d0e77
refactoring
sirasistant Jun 11, 2024
3180711
preparing for PR
sirasistant Jun 11, 2024
0a18f3a
build profiler bin
sirasistant Jun 11, 2024
dba410e
move script
sirasistant Jun 11, 2024
12370a7
print the path written in extractor
sirasistant Jun 11, 2024
a932bb1
update tests
sirasistant Jun 11, 2024
073039d
refactor: extract to a mock file
sirasistant Jun 11, 2024
aab0497
Merge branch 'master' into arv/flamegraph_acir
sirasistant Jun 11, 2024
553272a
update comment
sirasistant Jun 11, 2024
f1f2743
Merge branch 'arv/flamegraph_acir' of github.com:AztecProtocol/aztec-…
sirasistant Jun 11, 2024
f2eec54
conditional gate counting
sirasistant Jun 12, 2024
62b41e6
Merge branch 'master' into arv/flamegraph_acir
ludamad Jun 12, 2024
a48d748
better resizing
sirasistant Jun 12, 2024
86e008e
better conditioning of the gate tracking
sirasistant Jun 12, 2024
6f1a07e
use imported line and column helper
sirasistant Jun 12, 2024
7e778b2
add test
sirasistant Jun 12, 2024
ea459c7
fix: frame separator
sirasistant Jun 12, 2024
1137d53
rename binary
sirasistant Jun 13, 2024
2c361db
fix extraneous naming
sirasistant Jun 13, 2024
36f9b66
Merge branch 'master' into arv/flamegraph_acir
sirasistant Jun 13, 2024
b1a71e1
fmt
sirasistant Jun 13, 2024
1f48133
review changes
sirasistant Jun 13, 2024
5093fdd
fmt
sirasistant Jun 13, 2024
83293cc
Merge branch 'master' into arv/flamegraph_acir
sirasistant Jun 13, 2024
ead7db9
restore initializers
sirasistant Jun 13, 2024
66026fd
add initializer for gates per opcode
sirasistant Jun 13, 2024
d3f231c
address rust pr comments
sirasistant Jun 13, 2024
ba784f5
more pr comment addressing
sirasistant Jun 13, 2024
9cc4691
gcc constructors...
sirasistant Jun 13, 2024
b387e18
Merge branch 'master' into arv/flamegraph_acir
sirasistant Jun 13, 2024
7ae3e34
refactor: use debug artifact instead of implementing files
sirasistant Jun 13, 2024
f26fdfa
Merge branch 'master' into arv/flamegraph_acir
sirasistant Jun 13, 2024
ebe4985
revert formatting change
sirasistant Jun 13, 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
14 changes: 12 additions & 2 deletions barretenberg/cpp/src/barretenberg/bb/main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -446,15 +446,25 @@ void gateCount(const std::string& bytecodePath, bool honk_recursion)
size_t i = 0;
for (auto constraint_system : constraint_systems) {
acir_proofs::AcirComposer acir_composer(0, verbose_logging);
acir_composer.create_circuit(constraint_system);
acir_composer.create_circuit(constraint_system, {}, true);
Copy link
Contributor

Choose a reason for hiding this comment

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

A question also for other reviewers: in general I recommend adding /*param=*/{} for any literal value or when the param name is not clear. This does wonders for readability (e.g. here, reviewing). However, VS already fills in the name for you.

What's the recommendation? (from my side, I think I still prefer adding the comment)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I typically don't do it for languages that have language servers that provide this info, but as you guys prefer

auto circuit_size = acir_composer.get_total_circuit_size();

// Build individual circuit report
std::string gates_per_opcode_str;
for (size_t j = 0; j < constraint_system.gates_per_opcode.size(); j++) {
gates_per_opcode_str += std::to_string(constraint_system.gates_per_opcode[j]);
if (j != constraint_system.gates_per_opcode.size() - 1) {
gates_per_opcode_str += ",";
}
}

auto result_string = format("{\n \"acir_opcodes\": ",
constraint_system.num_acir_opcodes,
",\n \"circuit_size\": ",
circuit_size,
"\n }");
",\n \"gates_per_opcode\": [",
gates_per_opcode_str,
"]\n }");

// Attach a comma if we still circuit reports to generate
if (i != (constraint_systems.size() - 1)) {
Expand Down
191 changes: 153 additions & 38 deletions barretenberg/cpp/src/barretenberg/dsl/acir_format/acir_format.cpp

Large diffs are not rendered by default.

64 changes: 56 additions & 8 deletions barretenberg/cpp/src/barretenberg/dsl/acir_format/acir_format.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,44 @@

namespace acir_format {

/**
* @brief Indices of the original opcode that originated each constraint in AcirFormat.
* @details Contains one array of indices per opcode type. The length of each array is equal to the number of
* constraints of that type. The relationship between the opcodes and constraints is assumed to be one to one, except
* for block constraints.
*/
struct AcirFormatOriginalOpcodeIndices {
std::vector<size_t> logic_constraints;
std::vector<size_t> range_constraints;
std::vector<size_t> aes128_constraints;
std::vector<size_t> sha256_constraints;
std::vector<size_t> sha256_compression;
std::vector<size_t> schnorr_constraints;
std::vector<size_t> ecdsa_k1_constraints;
std::vector<size_t> ecdsa_r1_constraints;
std::vector<size_t> blake2s_constraints;
std::vector<size_t> blake3_constraints;
std::vector<size_t> keccak_constraints;
std::vector<size_t> keccak_permutations;
std::vector<size_t> pedersen_constraints;
std::vector<size_t> pedersen_hash_constraints;
std::vector<size_t> poseidon2_constraints;
std::vector<size_t> multi_scalar_mul_constraints;
std::vector<size_t> ec_add_constraints;
std::vector<size_t> recursion_constraints;
std::vector<size_t> honk_recursion_constraints;
std::vector<size_t> bigint_from_le_bytes_constraints;
std::vector<size_t> bigint_to_le_bytes_constraints;
std::vector<size_t> bigint_operations;
std::vector<size_t> poly_triple_constraints;
std::vector<size_t> quad_constraints;
// Multiple opcode indices per block:
std::vector<std::vector<size_t>> block_constraints;

friend bool operator==(AcirFormatOriginalOpcodeIndices const& lhs,
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you need to be explicit on == ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If I didn't do this the parent struct complains about Explicitly defaulted equality comparison operator is implicitly deletedclang(-Wdefaulted-function-deleted)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

IDK why the parent is explicit though

AcirFormatOriginalOpcodeIndices const& rhs) = default;
};

struct AcirFormat {
// The number of witnesses in the circuit
uint32_t varnum;
Expand Down Expand Up @@ -72,6 +110,13 @@ struct AcirFormat {
quad_constraints;
std::vector<BlockConstraint> block_constraints;

// Number of gates added to the circuit per original opcode.
// Has length equal to num_acir_opcodes.
std::vector<size_t> gates_per_opcode;

// Indices of the original opcode that originated each constraint in AcirFormat.
AcirFormatOriginalOpcodeIndices original_opcode_indices;

// For serialization, update with any new fields
MSGPACK_FIELDS(varnum,
public_inputs,
Expand Down Expand Up @@ -142,18 +187,21 @@ struct AcirProgramStack {
};

template <typename Builder = bb::UltraCircuitBuilder>
Builder create_circuit(const AcirFormat& constraint_system,
Builder create_circuit(AcirFormat& constraint_system,
size_t size_hint = 0,
WitnessVector const& witness = {},
bool honk_recursion = false,
std::shared_ptr<bb::ECCOpQueue> op_queue = std::make_shared<bb::ECCOpQueue>());
std::shared_ptr<bb::ECCOpQueue> op_queue = std::make_shared<bb::ECCOpQueue>(),
bool collect_gates_per_opcode = false);

template <typename Builder>
void build_constraints(Builder& builder,
AcirFormat const& constraint_system,
bool has_valid_witness_assignments,
bool honk_recursion = false); // honk_recursion means we will honk to recursively verify this
// circuit. This distinction is needed to not add the default
// aggregation object when we're not using the honk RV.
void build_constraints(
Builder& builder,
AcirFormat& constraint_system,
bool has_valid_witness_assignments,
bool honk_recursion = false,
bool collect_gates_per_opcode = false); // honk_recursion means we will honk to recursively verify this
// circuit. This distinction is needed to not add the default
// aggregation object when we're not using the honk RV.

} // namespace acir_format
Loading
Loading