- Adding iterator for read-only buffers to optimize for sequential reads.

- Small changes for code review.

Author: avalerio-tkd

src/processing/byte_buffer.cpp

@@ -134,15 +134,15 @@ size_t ByteBuffer::EstimateOffsetsReserveCountFromSample(tcb::span<const uint8_t
         ++sampled_elements;
     }
 
+    // If no elements were sampled or the cursor didn't move, it means the buffer is empty. So return 0.
     if (sampled_elements == 0 || cursor == 0)
         return 0;
 
     // If sampling consumed the full buffer (<= sample window), we already know the exact count.
     if (cursor == bytes.size())
         return sampled_elements;
 
-    // Estimate total element count from sample density:
-    //   (sampled_elements / sampled_bytes) * total_bytes.
+    // Estimate total element count from sample density: (sampled_elements / sampled_bytes) * total_bytes.
     // - sampled_elements / sampled_bytes gives "elements per byte" in the sampled prefix,
     // - then multiplying by total_bytes extrapolates a full-buffer estimate.
     const long double estimated =
@@ -151,11 +151,13 @@ size_t ByteBuffer::EstimateOffsetsReserveCountFromSample(tcb::span<const uint8_t
     const size_t estimated_count = static_cast<size_t>(std::ceil(estimated));
     const size_t estimated_with_headroom =
         static_cast<size_t>(std::ceil(static_cast<long double>(estimated_count) * 1.1L));
+
+    // If the count of sampled elements is more than the estimated, conservatively return actual count.
     return std::max(estimated_with_headroom, sampled_elements);
 }
 
 // -----------------------------------------------------------------------------
-// Span reader methods
+// Element span reader methods
 // -----------------------------------------------------------------------------
 
 size_t ByteBuffer::CalculateOffsetOfElement(size_t position) const {
@@ -189,6 +191,99 @@ tcb::span<const uint8_t> ByteBuffer::GetElement(size_t position) const {
     return elements_span_.subspan(offset + kSizePrefixBytes, element_size);
 }
 
+// -----------------------------------------------------------------------------
+// Elemment span iterator
+//
+// Allows an alternative read of elements_span_ without need for lazy initialization of offsets_,
+// so saving execution time when the traversal of the buffer is strictly sequential.
+// This is the most common behavior when reading elements in single threaded mode.
+// -----------------------------------------------------------------------------
+
+ByteBuffer::ConstIterator::ConstIterator(const ByteBuffer* buffer, size_t cursor_offset)
+    : buffer_(buffer), cursor_offset_(cursor_offset) {}
+
+void ByteBuffer::ConstIterator::ValidateFixedSizeElementAtCursor() const {
+    if (buffer_->element_size_ <= 0) {
+        throw InvalidInputException("Invalid fixed-size buffer: element_size must be greater than zero");
+    }
+    if ((buffer_->elements_span_.size() - cursor_offset_) < buffer_->element_size_) {
+        throw InvalidInputException("Malformed fixed-size buffer: truncated element payload");
+    }
+}
+
+size_t ByteBuffer::ConstIterator::ReadAndValidateVariableElementSizeAtCursor() const {
+    if ((buffer_->elements_span_.size() - cursor_offset_) < kSizePrefixBytes) {
+        throw InvalidInputException("Malformed variable-size buffer: truncated length prefix");
+    }
+    const size_t current_element_size = ReadSizeAt(buffer_->elements_span_, cursor_offset_);
+    const size_t payload_offset = cursor_offset_ + kSizePrefixBytes;
+    if ((buffer_->elements_span_.size() - payload_offset) < current_element_size) {
+        throw InvalidInputException("Malformed variable-size buffer: truncated element payload");
+    }
+    return current_element_size;
+}
+
+ByteBuffer::ConstIterator::value_type ByteBuffer::ConstIterator::operator*() const {
+    if (buffer_ == nullptr || cursor_offset_ >= buffer_->elements_span_.size()) {
+        throw InvalidInputException("Cannot dereference ByteBuffer iterator at end position");
+    }
+    if (buffer_->has_fixed_sized_elements_) {
+        ValidateFixedSizeElementAtCursor();
+        return buffer_->elements_span_.subspan(cursor_offset_, buffer_->element_size_);
+    }
+
+    const size_t current_element_size = ReadAndValidateVariableElementSizeAtCursor();
+    const size_t payload_offset = cursor_offset_ + kSizePrefixBytes;
+    return buffer_->elements_span_.subspan(payload_offset, current_element_size);
+}
+
+ByteBuffer::ConstIterator& ByteBuffer::ConstIterator::operator++() {
+    if (buffer_ == nullptr || cursor_offset_ >= buffer_->elements_span_.size()) {
+        return *this;
+    }
+    if (buffer_->has_fixed_sized_elements_) {
+        ValidateFixedSizeElementAtCursor();
+        cursor_offset_ += buffer_->element_size_;
+        return *this;
+    }
+
+    const size_t current_element_size = ReadAndValidateVariableElementSizeAtCursor();
+    cursor_offset_ += (kSizePrefixBytes + current_element_size);
+    return *this;
+}
+
+bool ByteBuffer::ConstIterator::operator==(const ConstIterator& other) const {
+    return buffer_ == other.buffer_ && cursor_offset_ == other.cursor_offset_;
+}
+
+bool ByteBuffer::ConstIterator::operator!=(const ConstIterator& other) const {
+    return !(*this == other);
+}
+
+void ByteBuffer::ValidateIteratorReadPreconditions() const {
+    if (is_write_buffer_initialized_) {
+        throw InvalidInputException("Iterator is only available for read buffers");
+    }
+    if (has_fixed_sized_elements_) {
+        if (element_size_ <= 0) {
+            throw InvalidInputException("Invalid fixed-size buffer: element_size must be greater than zero");
+        }
+        if ((elements_span_.size() % element_size_) != 0) {
+            throw InvalidInputException("Malformed fixed-size buffer: buffer does not align with element_size");
+        }
+    }
+}
+
+ByteBuffer::ConstIterator ByteBuffer::begin() const {
+    ValidateIteratorReadPreconditions();
+    return ConstIterator(this, 0u);
+}
+
+ByteBuffer::ConstIterator ByteBuffer::end() const {
+    ValidateIteratorReadPreconditions();
+    return ConstIterator(this, elements_span_.size());
+}
+
 // -----------------------------------------------------------------------------
 // Constructors and initializers for write buffer
 // -----------------------------------------------------------------------------

src/processing/byte_buffer.h

@@ -19,6 +19,7 @@
 
 #include <cstddef>
 #include <cstdint>
+#include <iterator>
 #include <limits>
 #include <vector>
 
@@ -57,13 +58,48 @@ class ByteBuffer {
     // Finalizes the write path and transfers the resulting buffer ownership.
     std::vector<uint8_t> FinalizeAndTakeBuffer();
 
+    // Iterator for read-only elements.
+    class ConstIterator {
+        public:
+            // Iterator traits consumed indirectly by STL iterator machinery.
+            using iterator_category = std::forward_iterator_tag;
+            using value_type = tcb::span<const uint8_t>;
+            using difference_type = std::ptrdiff_t;
+            using pointer = void;
+            using reference = value_type;
+    
+            // Basic forward-iterator operations over encoded elements in elements_span_.
+            ConstIterator(const ByteBuffer* buffer, size_t cursor_offset);
+            value_type operator*() const;
+            ConstIterator& operator++();
+            bool operator==(const ConstIterator& other) const;
+            bool operator!=(const ConstIterator& other) const;
+    
+        private:
+            void ValidateFixedSizeElementAtCursor() const;
+            size_t ReadAndValidateVariableElementSizeAtCursor() const;
+
+            const ByteBuffer* buffer_ = nullptr;
+            size_t cursor_offset_ = 0;
+        };
+    
+    // Methods used by the STL iterator machinery to iterate over the buffer.
+    ConstIterator begin() const;
+    ConstIterator end() const;
+
 protected:
     // Helper for reserve heuristics in variable-size parsing.
     static size_t EstimateOffsetsReserveCountFromSample(tcb::span<const uint8_t> bytes);
 
     // Helper for calculating the offset of an element by position.
     size_t CalculateOffsetOfElement(size_t position) const;
 
+    // Helper to validate the preconditions for reading the buffer with an iterator.
+    void ValidateIteratorReadPreconditions() const;
+
+    // Helper to check if the buffer is initialized from a span.
+    bool IsInitializedFromSpan() const { return is_initialized_from_span_; }
+
     // Variables for span elements reading
     tcb::span<const uint8_t> elements_span_;
     mutable size_t num_elements_;
@@ -94,7 +130,10 @@ class ByteBuffer {
     bool is_write_buffer_finalized_ = false;    
 };
 
+// Constant to mark an offset value as unset.
 inline constexpr size_t kUnsetVariableElementOffset = std::numeric_limits<size_t>::max();
-inline constexpr size_t kSizePrefixBytes = sizeof(uint32_t);  // [u32 size] prefix for variable-size elements.
+
+// Constant for the size of the [u32 size] prefix for variable-size elements.
+inline constexpr size_t kSizePrefixBytes = sizeof(uint32_t);
 
 } // namespace dbps::processing

src/processing/byte_buffer_test.cpp

@@ -51,6 +51,7 @@ class ByteBufferTestProxy : public ByteBuffer {
     bool GetHasFixedSizedElements() const { return has_fixed_sized_elements_; }
     size_t GetElementSize() const { return element_size_; }
     const std::vector<size_t>& GetOffsets() const { return offsets_; }
+    bool GetIsInitializedFromSpan() const { return IsInitializedFromSpan(); }
     const std::vector<uint8_t>& GetWriteBuffer() const { return write_buffer_; }
     void AppendTrailingBytesForTest(tcb::span<const uint8_t> bytes) {
         write_buffer_.insert(write_buffer_.end(), bytes.begin(), bytes.end());
@@ -207,6 +208,45 @@ TEST(ByteBufferTest, GetElement_VariableSize_ReturnsExpectedPayload) {
     EXPECT_EQ(second[6], 0x37);
 }
 
+TEST(ByteBufferTest, Iterate_ReadOnlyFixedSize_TraversesInOrder) {
+    std::vector<uint8_t> bytes = {0x10, 0x11, 0x20, 0x21, 0x30, 0x31};
+    ByteBufferTestProxy buffer(tcb::span<const uint8_t>(bytes), 2);
+
+    std::vector<std::vector<uint8_t>> collected;
+    for (const auto element : buffer) {
+        collected.push_back(std::vector<uint8_t>(element.begin(), element.end()));
+    }
+
+    ASSERT_EQ(collected.size(), 3u);
+    EXPECT_EQ(collected[0], (std::vector<uint8_t>{0x10, 0x11}));
+    EXPECT_EQ(collected[1], (std::vector<uint8_t>{0x20, 0x21}));
+    EXPECT_EQ(collected[2], (std::vector<uint8_t>{0x30, 0x31}));
+    EXPECT_TRUE(buffer.GetOffsets().empty());
+    EXPECT_EQ(buffer.GetNumElements(), 0u);
+    EXPECT_FALSE(buffer.GetIsInitializedFromSpan());
+}
+
+TEST(ByteBufferTest, Iterate_ReadOnlyVariableSize_TraversesInOrder) {
+    // [len=5]["ABCDE"][len=7]["1234567"]
+    std::vector<uint8_t> bytes = {
+        0x05, 0x00, 0x00, 0x00, 0x41, 0x42, 0x43, 0x44, 0x45,
+        0x07, 0x00, 0x00, 0x00, 0x31, 0x32, 0x33, 0x34, 0x35, 0x36, 0x37
+    };
+    ByteBufferTestProxy buffer{tcb::span<const uint8_t>(bytes)};
+
+    std::vector<std::vector<uint8_t>> collected;
+    for (const auto element : buffer) {
+        collected.push_back(std::vector<uint8_t>(element.begin(), element.end()));
+    }
+
+    ASSERT_EQ(collected.size(), 2u);
+    EXPECT_EQ(collected[0], (std::vector<uint8_t>{0x41, 0x42, 0x43, 0x44, 0x45}));
+    EXPECT_EQ(collected[1], (std::vector<uint8_t>{0x31, 0x32, 0x33, 0x34, 0x35, 0x36, 0x37}));
+    EXPECT_TRUE(buffer.GetOffsets().empty());
+    EXPECT_EQ(buffer.GetNumElements(), 0u);
+    EXPECT_FALSE(buffer.GetIsInitializedFromSpan());
+}
+
 TEST(ByteBufferTest, GetElement_OutOfRange_Throws) {
     std::vector<uint8_t> bytes = {0x01, 0x02, 0x03, 0x04};
     ByteBufferTestProxy buffer(tcb::span<const uint8_t>(bytes), 2);
@@ -600,6 +640,12 @@ TEST(ByteBufferTest, SetElement_OnReadOnlyBuffer_Throws) {
     EXPECT_THROW((void)buffer.SetElement(0, tcb::span<const uint8_t>(replacement)), InvalidInputException);
 }
 
+TEST(ByteBufferTest, Iterate_OnWriteBuffer_Throws) {
+    ByteBufferTestProxy buffer(3u, 2u);
+    EXPECT_THROW((void)buffer.begin(), InvalidInputException);
+    EXPECT_THROW((void)buffer.end(), InvalidInputException);
+}
+
 TEST(ByteBufferTest, ConstructVariableSize_EmptyBuffer_InitializesEmptyState) {
     std::vector<uint8_t> bytes;
     ByteBufferTestProxy buffer{tcb::span<const uint8_t>(bytes)};