chore: sync public-v5-next with upstream v5-next

Author: AztecBot

.test_patterns.yml

@@ -410,11 +410,6 @@ tests:
     owners:
       - *martin
 
-  - regex: "yarn-project/scripts/run_test.sh bb-prover/src/avm_proving_tests/avm_"
-    error_regex: "timeout: sending signal"
-    owners:
-      - *charlie
-
   - regex: "run_test.sh simple tx_stats_bench"
     error_regex: "✕ verifies transactions at 10 TPS"
     owners:

barretenberg/cpp/src/barretenberg/vm2/tracegen/lib/lookup_builder.hpp

@@ -51,10 +51,10 @@ template <typename LookupSettings_> class IndexedLookupTraceBuilder : public Int
             }
 
             trace.set(LookupSettings::COUNTS, dst_row, trace.get(LookupSettings::COUNTS, dst_row) + 1);
-            // Set the fine grained inner selector if it's not already one.
-            if (LookupSettings::DST_SELECTOR != this->outer_dst_selector &&
-                trace.get(LookupSettings::DST_SELECTOR, dst_row) != 1) {
-                trace.set(LookupSettings::DST_SELECTOR, dst_row, 1);
+            if (LookupSettings::DST_SELECTOR != this->outer_dst_selector) {
+                // This step might write to the same cell from multiple threads, so we use atomic limbs to avoid UB.
+                // Since we are always writing a 1, the end result will be 1 even under concurrency.
+                trace.set(LookupSettings::DST_SELECTOR, dst_row, 1, /*use_atomic_limbs=*/true);
             }
         });
     }
@@ -198,10 +198,10 @@ template <typename LookupSettings> class LookupIntoDynamicTableSequential : publ
                 if (dst_selector == 1 && src_values == trace.get_multiple(LookupSettings::DST_COLUMNS, dst_row)) {
                     trace.set(LookupSettings::COUNTS, dst_row, trace.get(LookupSettings::COUNTS, dst_row) + 1);
 
-                    // Set the fine grained inner selector if it's not already one.
-                    if (LookupSettings::DST_SELECTOR != this->outer_dst_selector &&
-                        trace.get(LookupSettings::DST_SELECTOR, dst_row) != 1) {
-                        trace.set(LookupSettings::DST_SELECTOR, dst_row, 1);
+                    if (LookupSettings::DST_SELECTOR != this->outer_dst_selector) {
+                        // This step might write to the same cell from multiple threads, so we use atomic limbs to avoid
+                        // UB. Since we are always writing a 1, the end result will be 1 even under concurrency.
+                        trace.set(LookupSettings::DST_SELECTOR, dst_row, 1, /*use_atomic_limbs=*/true);
                     }
 
                     found = true;

barretenberg/cpp/src/barretenberg/vm2/tracegen/trace_container.cpp

@@ -1,8 +1,10 @@
 #include "barretenberg/vm2/tracegen/trace_container.hpp"
 
 #include <algorithm>
-#include <mutex>
+#include <ranges>
 
+#include "barretenberg/common/assert.hpp"
+#include "barretenberg/common/compiler_hints.hpp"
 #include "barretenberg/common/log.hpp"
 #include "barretenberg/common/ref_vector.hpp"
 #include "barretenberg/vm2/common/field.hpp"
@@ -12,7 +14,19 @@ namespace bb::avm2::tracegen {
 namespace {
 
 // We need a zero value to return (a reference to) when a value is not found.
-static const FF zero = FF::zero();
+const FF zero = FF::zero();
+
+// Writes each 64-bit limb of the field with a relaxed atomic store. Each limb is naturally aligned (FF is
+// alignas(32), 4x uint64_t), so this lowers to 4 plain `movq` stores on x86-64 — no lock, no libatomic call
+// (unlike a whole-field std::atomic_ref<FF>, whose 32 bytes exceed the lock-free width). Lets set() make a
+// same-cell concurrent write data-race-free when every writer stores the same value.
+inline void store_per_limb(FF& cell, const FF& value)
+{
+    static_assert(sizeof(FF) == 4 * sizeof(uint64_t));
+    for (size_t i = 0; i < 4; ++i) {
+        std::atomic_ref<uint64_t>(cell.data[i]).store(value.data[i], std::memory_order_relaxed);
+    }
+}
 
 } // namespace
 
@@ -23,9 +37,15 @@ TraceContainer::TraceContainer()
 const FF& TraceContainer::get(Column col, uint32_t row) const
 {
     auto& column_data = (*trace)[static_cast<size_t>(col)];
-    std::shared_lock lock(column_data.mutex);
-    const auto it = column_data.rows.find(row);
-    return it == column_data.rows.end() ? zero : it->second;
+    const size_t shard_idx = row / INTERVAL_SIZE;
+    if (shard_idx >= NUM_SHARDS) {
+        return zero;
+    }
+    ColumnInterval* shard = column_data.slots[shard_idx].load(std::memory_order_acquire);
+    if (shard == nullptr) {
+        return zero;
+    }
+    return shard->rows[row % INTERVAL_SIZE];
 }
 
 const FF& TraceContainer::get_column_or_shift(ColumnAndShifts col, uint32_t row) const
@@ -36,19 +56,52 @@ const FF& TraceContainer::get_column_or_shift(ColumnAndShifts col, uint32_t row)
     return get(static_cast<Column>(col), row);
 }
 
-void TraceContainer::set(Column col, uint32_t row, const FF& value)
+TraceContainer::ColumnInterval& TraceContainer::get_or_create_shard(SparseColumn& column_data, size_t shard_idx)
+{
+    ColumnInterval* shard = column_data.slots[shard_idx].load(std::memory_order_acquire);
+    if (shard != nullptr) {
+        return *shard;
+    }
+    // Slow path: this slot has never been written. Construct a shard and install it with a single CAS.
+    // Creators of different shards target different atomics and run fully in parallel with no lock. If we
+    // lose the race for this slot (only possible when two chunks share a shard at a boundary), we discard
+    // our spare copy and use the winner's.
+    auto fresh = std::make_unique<ColumnInterval>();
+    ColumnInterval* expected = nullptr;
+    if (column_data.slots[shard_idx].compare_exchange_strong(
+            expected, fresh.get(), std::memory_order_acq_rel, std::memory_order_acquire)) {
+        return *fresh.release();
+    }
+    return *expected; // CAS failure loaded the winning pointer into `expected` (acquire).
+}
+
+void TraceContainer::set(Column col, uint32_t row, const FF& value, bool use_atomic_limbs)
 {
     auto& column_data = (*trace)[static_cast<size_t>(col)];
-    std::unique_lock lock(column_data.mutex);
+    const size_t shard_idx = row / INTERVAL_SIZE;
+    BB_ASSERT_LT(shard_idx, NUM_SHARDS, "row exceeds the maximum trace size");
+    const uint32_t offset = row % INTERVAL_SIZE;
+
     if (!value.is_zero()) {
-        column_data.rows.insert_or_assign(row, value);
-        column_data.max_row_number = std::max(column_data.max_row_number, static_cast<int64_t>(row));
+        // Lock-free: a single atomic load finds the shard (created on first write), then we write our
+        // own dense cell directly. Different rows are distinct array elements, so concurrent writers of
+        // this column (or even of the same shard, at a chunk boundary) never race and never serialize.
+        auto& cell = get_or_create_shard(column_data, shard_idx).rows[offset];
+        if (BB_UNLIKELY(use_atomic_limbs)) {
+            store_per_limb(cell, value);
+        } else {
+            cell = value;
+        }
     } else {
-        auto num_erased = column_data.rows.erase(row);
-        if (column_data.max_row_number == row && num_erased > 0) {
-            // This shouldn't happen often. We delay recalculation of the max row number
-            // until someone actually needs it.
-            column_data.row_number_dirty = true;
+        // Zero value: clear if present. We never create a shard, so sparse (mostly-zero) columns are not
+        // materialized (an unset cell already reads as zero).
+        ColumnInterval* shard = column_data.slots[shard_idx].load(std::memory_order_acquire);
+        if (shard != nullptr) {
+            if (BB_UNLIKELY(use_atomic_limbs)) {
+                store_per_limb(shard->rows[offset], zero);
+            } else {
+                shard->rows[offset] = FF::zero();
+            }
         }
     }
 }
@@ -62,24 +115,39 @@ void TraceContainer::set(uint32_t row, std::span<const std::pair<Column, FF>> va
 
 void TraceContainer::reserve_column(Column col, size_t size)
 {
+    if (size == 0) {
+        return;
+    }
     auto& column_data = (*trace)[static_cast<size_t>(col)];
-    std::unique_lock lock(column_data.mutex);
-    column_data.rows.reserve(size);
+    const size_t num_shards = std::min((size + INTERVAL_SIZE - 1) / INTERVAL_SIZE, NUM_SHARDS);
+    // Each shard's dense row array is full size on creation, so reserving just materializes the shards up
+    // front (e.g. for precomputed columns). Lock-free: get_or_create_shard installs each via CAS.
+    for (size_t k = 0; k < num_shards; ++k) {
+        get_or_create_shard(column_data, k);
+    }
 }
 
 uint32_t TraceContainer::get_column_rows(Column col) const
 {
+    // The number of rows is (highest non-zero absolute row + 1). We find it by scanning shards from the
+    // top down and, within the first non-empty shard, scanning its rows from the top. Shards are
+    // top-dense, so this terminates almost immediately. This is only called after the parallel fill
+    // phase, so no lock is needed. Lower shards cannot hold a higher row, so the first hit is the answer.
     auto& column_data = (*trace)[static_cast<size_t>(col)];
-    std::unique_lock lock(column_data.mutex);
-    if (column_data.row_number_dirty) {
-        // Trigger recalculation of max row number.
-        auto keys = std::views::keys(column_data.rows);
-        const auto it = std::ranges::max_element(keys);
-        // We use -1 to indicate that the column is empty.
-        column_data.max_row_number = it == keys.end() ? -1 : static_cast<int64_t>(*it);
-        column_data.row_number_dirty = false;
-    }
-    return static_cast<uint32_t>(column_data.max_row_number + 1);
+    for (size_t k = NUM_SHARDS; k-- > 0;) {
+        ColumnInterval* shard_ptr = column_data.slots[k].load(std::memory_order_acquire);
+        if (shard_ptr == nullptr) {
+            continue;
+        }
+        const auto& rows = shard_ptr->rows;
+        const uint32_t base = static_cast<uint32_t>(k) * INTERVAL_SIZE;
+        for (uint32_t off = INTERVAL_SIZE; off-- > 0;) {
+            if (!rows[off].is_zero()) {
+                return base + off + 1;
+            }
+        }
+    }
+    return 0;
 }
 
 uint32_t TraceContainer::get_num_witness_rows() const
@@ -103,9 +171,18 @@ uint32_t TraceContainer::get_num_rows() const
 void TraceContainer::visit_column(Column col, const std::function<void(uint32_t, const FF&)>& visitor) const
 {
     auto& column_data = (*trace)[static_cast<size_t>(col)];
-    std::shared_lock lock(column_data.mutex);
-    for (const auto& [row, value] : column_data.rows) {
-        visitor(row, value);
+    for (size_t k = 0; k < NUM_SHARDS; ++k) {
+        ColumnInterval* shard_ptr = column_data.slots[k].load(std::memory_order_acquire);
+        if (shard_ptr == nullptr) {
+            continue;
+        }
+        auto& shard = *shard_ptr;
+        const uint32_t base = static_cast<uint32_t>(k) * INTERVAL_SIZE;
+        for (uint32_t off = 0; off < INTERVAL_SIZE; ++off) {
+            if (!shard.rows[off].is_zero()) {
+                visitor(base + off, shard.rows[off]);
+            }
+        }
     }
 }
 
@@ -120,20 +197,28 @@ void TraceContainer::invert_column(Column col)
 {
     RefVector<FF> ff_vector;
     auto& column_data = (*trace)[static_cast<size_t>(col)];
-    std::unique_lock lock(column_data.mutex);
-    for (auto& [row, value] : column_data.rows) {
-        ff_vector.push_back(value);
+    for (size_t k = 0; k < NUM_SHARDS; ++k) {
+        ColumnInterval* shard_ptr = column_data.slots[k].load(std::memory_order_acquire);
+        if (shard_ptr == nullptr) {
+            continue;
+        }
+        auto& shard = *shard_ptr;
+        for (auto& value : shard.rows) {
+            if (!value.is_zero()) {
+                ff_vector.push_back(value);
+            }
+        }
     }
     FF::batch_invert<RefVector<FF>>(ff_vector);
 }
 
 void TraceContainer::clear_column(Column col)
 {
+    // Lock-free: exchange hands each non-null shard pointer to exactly one caller, which frees it.
     auto& column_data = (*trace)[static_cast<size_t>(col)];
-    std::unique_lock lock(column_data.mutex);
-    column_data.rows.clear();
-    column_data.max_row_number = -1;
-    column_data.row_number_dirty = false;
+    for (size_t k = 0; k < NUM_SHARDS; ++k) {
+        delete column_data.slots[k].exchange(nullptr, std::memory_order_acq_rel);
+    }
 }
 
 } // namespace bb::avm2::tracegen