Re: [boost] boost.lockfree update

Subject: Re: [boost] boost.lockfree update
From: Tim Blechmann (tim_at_[hidden])
Date: 2010-07-23 15:08:14

hi christian,

> This looks like a really useful addition to boost, hope you'll push it for
> review at some point.

it has been on the review queue for a while, but with the current review 
speed, it may take some time :(

> Some quick feedback on the fifo class
> 
> Fifo:
> "The fifo class provides a multi-writer/multi-reader fifo, enqueueing and
> dequeueing is lockfree, construction/destruction has to be synchronized"
> 
> What does it mean that construction/destruction has to be synchronized?
> Both these functions can only be called once.

one shouldn't try to en/dequeue from one thread, while another thread is 
running the constructor/destuctor. maybe it should formulated in a better 
way ...

>     fifo(void):
>         pool(128)
>     {
>         initialize();
>     }
> 
> I would prefer having the default constructor to not allocate any memory
> at all. Why not expose a reserve() member function instead?

good point ...

> bool enqueue(T const & t);
> Shouldn't this function throw an exception if t cannot be enqueued?

i would prefer not to throw an exception! if a constant-size freelist is 
used, the enqueue operation may fail, although no `error' occurred. i prefer 
this interface

> Can the name be changed to push_back() instead, to mimic other
> std::containers? One may then, for instance, use your class with
> std::back_inserter
> 
> Suggestion
> void push_back(const T&) // Throws...
> 
> 
> bool dequeue (T * ret)
> I think T should be taken by reference instead of pointer:
> Suggestion:
> bool pop_front(T&)
> or
> try_pop_front(T&)

hm ... enqueue/dequeue doesn't define a front or back ... not sure, whether 
one enqueues at the back and dequeues the back ...
maybe simple push/pop is also fine ... i am open for suggestion about this 
... taking a reference as argument is a good point!

> bool empty(void) //warning Not thread-safe
> I think this function should be removed from public interface if it cannot
> be implemented in a thread safe manner.

i would prefer to keep it, but warn about the thread safety. i am also 
thinking about adding thread-unsafe version of enqueue/dequeue/push/pop, 
since they can be implemented in a more efficient way than the thread-safe 
versions.
one can think of a use case, that one thread fills a fifo/queue and then 
notifies consumer threads ...

tim

-- 
tim_at_[hidden]
http://tim.klingt.org
Im übrigen ist es gescheiter, sich warm zuzudecken als sich zu
betrinken.
  Werner Schwab