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

fix: bump lookup table constant and improve logging #11095

Merged
merged 3 commits into from
Jan 8, 2025
Merged

fix: bump lookup table constant and improve logging #11095

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
890ca17
bump constant and improve logging
ledwards2225 Jan 7, 2025
60c7067
add todo
ledwards2225 Jan 7, 2025
50762fb
remove use of max lookups constant at the expense of memory
ledwards2225 Jan 8, 2025
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
3 changes: 2 additions & 1 deletion barretenberg/cpp/src/barretenberg/constants.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,8 @@ static constexpr uint32_t CONST_PG_LOG_N = 20;

static constexpr uint32_t CONST_ECCVM_LOG_N = 15;

static constexpr uint32_t MAX_LOOKUP_TABLES_SIZE = 75000;
// TODO(https://github.com/AztecProtocol/barretenberg/issues/1193): potentially reenable for better memory performance
// static constexpr uint32_t MAX_LOOKUP_TABLES_SIZE = 80000;

static constexpr uint32_t MAX_DATABUS_SIZE = 10000;

Expand Down
1 change: 1 addition & 0 deletions barretenberg/cpp/src/barretenberg/goblin/mock_circuits.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
#include "barretenberg/crypto/merkle_tree/merkle_tree.hpp"
#include "barretenberg/srs/global_crs.hpp"
#include "barretenberg/stdlib/encryption/ecdsa/ecdsa.hpp"
#include "barretenberg/stdlib/hash/keccak/keccak.hpp"
#include "barretenberg/stdlib/hash/sha256/sha256.hpp"
#include "barretenberg/stdlib/honk_verifier/ultra_recursive_verifier.hpp"
#include "barretenberg/stdlib/primitives/curves/secp256k1.hpp"
Expand Down
1 change: 0 additions & 1 deletion barretenberg/cpp/src/barretenberg/plonk_honk_shared/composer/composer_lib.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ void construct_lookup_table_polynomials(const RefArray<typename Flavor::Polynomi
// ignored, as used for regular constraints and padding to the next power of 2.
// TODO(https://github.com/AztecProtocol/barretenberg/issues/1033): construct tables and counts at top of trace
const size_t tables_size = circuit.get_tables_size();
ASSERT(tables_size <= MAX_LOOKUP_TABLES_SIZE); // if false, may need to increase constant
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 assert previously not being triggered?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In debug it would but otherwise I think not. Just rearranged so that theres some warning in release

ASSERT(dyadic_circuit_size > tables_size + additional_offset);
size_t offset = circuit.blocks.lookup.trace_offset;
if constexpr (IsPlonkFlavor<Flavor>) {
Expand Down
2 changes: 1 addition & 1 deletion barretenberg/cpp/src/barretenberg/ultra_honk/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
barretenberg_module(ultra_honk sumcheck stdlib_primitives)
barretenberg_module(ultra_honk sumcheck stdlib_primitives stdlib_keccak stdlib_sha256)
4 changes: 2 additions & 2 deletions barretenberg/cpp/src/barretenberg/ultra_honk/decider_proving_key.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,8 +99,8 @@ void DeciderProvingKey_<Flavor>::allocate_table_lookup_polynomials(const Circuit
PROFILE_THIS_NAME("allocate_table_lookup_polynomials");

size_t table_offset = circuit.blocks.lookup.trace_offset;
const size_t max_tables_size =
std::min(static_cast<size_t>(MAX_LOOKUP_TABLES_SIZE), dyadic_circuit_size - table_offset);
// TODO(https://github.com/AztecProtocol/barretenberg/issues/1193): can potentially improve memory footprint
const size_t max_tables_size = dyadic_circuit_size - table_offset;
ASSERT(dyadic_circuit_size > max_tables_size);

// Allocate the polynomials containing the actual table data
Expand Down
Loading