From 607734c60d517e4697253ace40dc7deeae937f41 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebasti=C3=A1n=20Mestre?= <36825825+SebastianMestre@users.noreply.github.com> Date: Thu, 27 Dec 2018 19:51:10 -0300 Subject: [PATCH] isoBufferBuffer refactor (#61) * Reimplements isoBufferBuffer and fixes a potential memory leak due to lack of a destructor --- Desktop_Interface/isobufferbuffer.cpp | 130 ++++++++++++++++++++------ Desktop_Interface/isobufferbuffer.h | 62 +++++++++--- 2 files changed, 151 insertions(+), 41 deletions(-) diff --git a/Desktop_Interface/isobufferbuffer.cpp b/Desktop_Interface/isobufferbuffer.cpp index b3a30489..e2d6e9fd 100644 --- a/Desktop_Interface/isobufferbuffer.cpp +++ b/Desktop_Interface/isobufferbuffer.cpp @@ -1,49 +1,127 @@ #include "isobufferbuffer.h" +#include + +/* isoBufferBuffer is implemented as two consecutive, duplicate, + * ring buffers. the effect of this is that we are able to hand + * out pointers to pieces of contiguous memory representing the + * last N inserted elements (for some N <= capacity()) even + * when we looped back to the begining of the ring buffer less + * than N insertions ago + * + * There are some differences with the original implementation + * functionality-wise. + * Where the original implementation allowed queries half as + * long as the length passed in, and allocated a buffer 1.5 times + * the length passed in, this version allows queries of length up + * to the length passed into the constructor and allocates a + * buffer 2 times the length passed in. + * Overall, this means that in contrast to the original + * implementation which only allowed queries up to a third as + * long as the allocated buffer, we can now do queries as long as + * half of the allocated buffer, which is a notable improvement. + */ + + +// TODO: go through the usages of this class and: +// 1. adapt code to use the new interface and remove the old one +// 2. adjust the size of the requested buffer to accomodate +// the improved memory efficiency of the new algorithm + isoBufferBuffer::isoBufferBuffer(uint32_t length) - : bufferLength(length) + : m_data(std::make_unique(length*2)) + , m_capacity(length) { - mid = bufferLength/2; - buffer = (char *) malloc((bufferLength * 3) / 2); } -void isoBufferBuffer::add(std::string newString) +// Adds a character to the end of the buffer +void isoBufferBuffer::insert(char c) { - for (char& newChar : newString) - add(newChar); + // Add character to first half of the buffer + m_data[m_top] = c; + // Then to the second + m_data[m_top+m_capacity] = c; + + // Loop the buffer index if necessary and update size accordingly + m_top = (m_top + 1) % m_capacity; + m_size = std::min(m_size + 1, m_capacity); +} + +void isoBufferBuffer::insert(char const * s) +{ + while (*s != '\0') + insert(*s++); +} + +void isoBufferBuffer::insert(std::string const & s) +{ + for (char c : s) + insert(c); +} + +char const* isoBufferBuffer::query(uint32_t count) const +{ + if (count > m_capacity) + qFatal("isoBufferBuffer::query : you may not request more items than the capacity of the buffer"); + + if (count > m_size) + qFatal("isoBufferBuffer::query : you may not request more items than inserted"); + + return end() - count; +} + +void isoBufferBuffer::clear() +{ + m_top = 0; + m_size = 0; +} + +char const * isoBufferBuffer::begin() const +{ + return m_data.get() + m_top - m_size + m_capacity; +} + +char const * isoBufferBuffer::end() const +{ + return m_data.get() + m_top + m_capacity; +} + +uint32_t isoBufferBuffer::size() const +{ + return m_size; +} + +uint32_t isoBufferBuffer::capacity() const +{ + return m_capacity; } -void isoBufferBuffer::add(char newChar){ - buffer[ptr] = newChar; +// Legacy Interface Implementation +void isoBufferBuffer::add(std::string const & newString) +{ + insert(newString); +} - if(ptr < mid) - buffer[ptr + bufferLength] = newChar; - if (ptr >= bufferLength) - ptr = 0; - else ptr++; - - numCharsInBuffer = std::min(numCharsInBuffer + 1, mid); +void isoBufferBuffer::add(char newChar) +{ + insert(newChar); } void isoBufferBuffer::add(uint8_t newByte) { - char newString[5]; - sprintf(newString, "0x%02hhx", newByte); - add(newString); + char newString[5]; + sprintf(newString, "0x%02hhx", newByte); + insert((char const *)newString); } uint32_t isoBufferBuffer::getNumCharsInBuffer() { - return numCharsInBuffer; + return size(); } -char *isoBufferBuffer::get(uint32_t length){ - if (length > mid) - qFatal("isoBuffer::get; length requested is too high."); - if(ptr < mid) - return &buffer[ptr + bufferLength - length]; - else - return &buffer[ptr - length]; +char const * isoBufferBuffer::get(uint32_t length) +{ + return query(length); } diff --git a/Desktop_Interface/isobufferbuffer.h b/Desktop_Interface/isobufferbuffer.h index e0dcd939..65b299c0 100644 --- a/Desktop_Interface/isobufferbuffer.h +++ b/Desktop_Interface/isobufferbuffer.h @@ -1,27 +1,59 @@ #ifndef ISOBUFFERBUFFER_H #define ISOBUFFERBUFFER_H -//isobufferbuffer is a buffer designed for getting the last n things added in reverse order, in O(1) time. - -#include -#include +#include #include +#include +/** @file isobufferbuffer.h + * @brief This module implements a data structure that allows + * insertion of single characters and a view of the last N + * inserted characters in constant time. + * + * To obtain such complexity, a double ring buffer is used. + * That is, two identical ring buffers are mantained adjacent + * in memory. If we always return a pointer to the beginning of a + * range that ends in the second buffer, we will always return a + * valid address(*), even when the requested length is greater + * than the current position being inserted into in the buffer. + * + * (*) By valid address I mean that both the addresses that + * represent the beginning and end of the requested query result + * are within the allocated buffer. + */ class isoBufferBuffer { public: - isoBufferBuffer(uint32_t length); - void add(uint8_t newByte); - void add(char newChar); - void add(std::string newString); - char *get(uint32_t length); - uint32_t getNumCharsInBuffer(); + isoBufferBuffer(uint32_t length); + ~isoBufferBuffer() = default; + + void insert(char c); + void insert(char const * s); + void insert(std::string const & s); + + char const * query(uint32_t length) const; + // TODO?: add ability to get a copy of the content + // (e.g. return std::string or Qstring) + + void clear(); + + char const * begin() const; + char const * end() const; + + uint32_t size() const; + uint32_t capacity() const; + + // Legacy Interface + void add(uint8_t newByte); + void add(char newChar); + void add(std::string const & newString); + char const *get(uint32_t length); + uint32_t getNumCharsInBuffer(); private: - uint32_t bufferLength; - uint32_t mid; - uint32_t ptr; - char *buffer; - uint32_t numCharsInBuffer = 0; + std::unique_ptr m_data; + uint32_t m_capacity; + uint32_t m_size = 0; + uint32_t m_top = 0; }; #endif // ISOBUFFERBUFFER_H