Re: [boost] Boost.Fiber review January 6-15

Subject: Re: [boost] Boost.Fiber review January 6-15
From: Oliver Kowalke (oliver.kowalke_at_[hidden])
Date: 2014-01-09 02:42:20

2014/1/8 Vicente J. Botet Escriba <vicente.botet_at_[hidden]>

> Hi Oliver,
>

hello Vicente

> The interface must at least follows the interface of the standard thread
> library and if there are some limitations, they must be explicitly
> documeted.
> Any difference respect Boost.Thread must also documented and the rational
> explained.
>

OK - section rational

> std::thread is not copyable by design, that is only one owner. WHy
> boost::fibers::fiber is copyable?
>

boost::fibers::fiber should be movable only - it is derived from
boost::noncopyable and uses BOOST_MOVABLE_BUT_NOT_COPYABLE

> Why the exceptions throw by the function given to a fiber is consumed by
> the framework instead of terminate the program as std::thread?
>

the trampoline-function used for the context does following:
- in a try-cactch-block execute the fiber-code (fiber-function given by the
user)
- catch exception forced_unwind from boost.coroutine -> release the fiber
and continue unwinding the stack
- catch fiber_interrupted -> stored inside boost::exception_ptr (might be
re-thrown in fiber::join() )
- catch all other exceptions and call std::terminate()

I though this would let to an equivalent behaviour as std::thread

> Which exception is thrown when the Error Conditions:resource_deadlock_would_occurand
> invalid_argument are signaled?
>

I use BOOST_ASSERT instead of exception

> Why priority and thread_affinity are not part of the fiber attributes?
>

you referre to class attributes passed to fiber's ctor? this class is a
vehicle for passing special parameter (for instance stack-size) to
boost::coroutine - if you use
the segmented-stack feature you usually don't need it.
priority() and thread_affinity() are member-functions of
boost::fibers::fiber to make the modifications those parameters for an
instance more explicit

> The interface let me think that the affinity can be changed by the owned
> of the thread and the thred itself. Is this by design?
>

thread_affinity() expresses if the fiber is bound to the thread - this is
required for fiber-stealing, e.g. a fiber which is bound to its running
thread
will not be selected as candidate for fiber-stealing.

> Please don't document get/set functions as thread_affinity altogether.
>

OK

> The safe_bool idiom should be replaced by a explict operator bool.
>

hmm - I thought using 'operator bool' is dangerous (I remind on some
discussion of this issue by Scott Meyers).
do you have other infos?

> Why is the scheduling algorithm global? Could it be threads specific?

It is thread specific (using boost::thread_specific_ptr)

> BTW i sthere an exmple showing  the |thread_specific_ptr trick mentioned
> on the documentation. |
>

which trick? maybe you referring to

algorithm *
scheduler::instance()
{
    if ( ! instance_.get() )
    {
        default_algo_.reset( new round_robin() );
        instance_.reset( default_algo_.get() );
    }
    return instance_.get();
}

> Why the time related function are limited to a specific clock?
>

it is a typedef to steady_clock (if avaliable) or system_clock - one of the
main reasons is that you would have made the schedulers be templates.

> The interface of fiber_group based on old and deprecated thread_group is
> not based on move semantics. Have you take a look at the proposal
> N3711 <http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2013/n3711.pdf>
> Task Groups As a Lower Level C++ Library Solution To Fork-Join Parallelism
>

no - but I've no problems to remove it form the library

> Maybe it is worth adapting it to fibers.
> **
> Boost.Thread has deprecated the use of the nested type scoped_lock as it
> introduce unnecessary dependencies. DO you think it is worth maintaining it?
>

oh - I wasn't aware of this issue - I've no preferrence to scoped_lock
(which is a typedef to unique_lock, AFAIK)

> I made some adaptations to boost::barrier that could also have a sens for
> fibers.

OK - what are those adaptations?

> I don't know if a single class could be defined that takes care of both
> contexts for high level classes as the barrier?
>

a problem is raised by the mutex implementations - thread's mutexes are
blocking the thread while fiber's mutexes do
only suspend the current fiber while keep the thread running (so other
fibers are able to run instead)

I was thinking on a combination of sync. primitives for threads and fibers
too , but it is not that easy to implement (with a clean interface)

> Boost.Thread would deliver two synchronized bounded and unbounded queue
> soon based on
> N3533 <http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2013/n3533.html>
> C++ Concurrent Queues
>

OK

> Have you tried to follow the same interface?
>

I did look at the proposal of C++ Concurrent Queues - but I didn't adapt
the complete interface.
for instance Element queue::value_pop(); -> queue_op_status queue::pop(
Element &);