[boost] Re: circular_buffer update

From: Pavel Vozenilek (pavel_vozenilek_at_[hidden])
Date: 2003-07-31 15:42:20

Hello Jano,

"Jan Gaspar" <jga_at_[hidden]> wrote
> The updated circular_buffer implementation can be found
> at the common place
> http://groups.yahoo.com/group/boost/files/circular_buffer.zip
>
I looked briefly over the code:

1. IMO the macro based exception handling isn't needed, it is better
    to use RAII, like:

void set_capacity(size_type new_capacity) {
  if (new_capacity == capacity()) return;
  pointer buff = allocate(new_capacity);
  size_type new_size = new_capacity < size() ? new_capacity : size();
  BOOST_CB_TRY
  std::uninitialized_copy(end() - new_size, end(), buff);
  BOOST_CB_UNWIND(deallocate(buff, new_capacity))
  destroy();
  m_size = new_size;
  m_buff = m_first = buff;
  m_end = m_buff + new_capacity;
   m_last = full() ? m_buff : m_buff + size();
}

can be replaced by:

void set_capacity(size_type new_capacity) {
  if (new_capacity == capacity()) return;
  pointer buff = allocate(new_capacity);

  struct deleter_t {
    pointer data;
   size_type capacity;
    deleter_t(pointer p, size_type s) : data(p), capacity(s) {}
    ~deleter_t() { deallocate(data, capacity); }
  };
  deleter_t guard(buff, new_capacity);

  size_type new_size = new_capacity < size() ? new_capacity : size();
  std::uninitialized_copy(end() - new_size, end(), buff);
  destroy();
  m_size = new_size;
  m_buff = m_first = buff;
  m_end = m_buff + new_capacity;
   m_last = full() ? m_buff : m_buff + size();
   guard.data = 0;
}

RAII may replace all the macros in the code. Source code size, runtime speed
and executable size should not get worse.

2. in function cb_iterator::operator -(), shouldn't it be std::less instead
    of less? (actually I do not see why < isn't enough here).

3. cb_iterator::operator +():

instead of

    cb_iterator operator + (difference_type n) const {
        cb_iterator tmp = *this;
        return tmp += n;
    }

you may use

    cb_iterator operator + (difference_type n) const {
        return cb_iterator (*this) += n;
    }

which looks the same but may use unnamed return value optimisation (URVO).
The original code would require compiler to support named RVO to optimize
copy away.

Some other code may or may not be changed in this way, I didn't look futher
for this.

4. circular_buffer::check_position(): raw throw  is used, it may be
conditionally
    disabled if  BOOST_NO_EXCEPTION or function from
    <boost/throw_exception.hpp> may be used.

5. Similarly in   circular_buffer::allocate(). (I wonder where
std::length_error
    does come from?)

> Regarding "unused space overhead"
>
> I share the Nigel's opinion. The circular_buffer was designed with fixed
> allocated memory. It will just complicate things. For example statements
> regarding iterator invalidation won't be true any more. On contrary it
> is quite easy to adapt std::list, std::slist or std::deque to achieve
> this goal. You can just push_back() elements at the end of e.g.
> std:list. In case the size of the container exceeds the desired capacity
> you just remove the element from the front.
>
Maybe in this case circular_buffer<> can reuse vector<> to keep internal
data
(the source code would be smaller + potential for smaller executable).

The circular_buffer<> could keep start+end pointers and delegate quite a lot
of functionality to std::vector<>.

(Since there are not many Mojo enabled vectors, using Mojo likely means
own buffer handling to be of any effect.)

/Pavel