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 14:28:57

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

> Then you should be more explicit in this paragraph
> "Exceptions thrown by the function or callable object passed to the
> |fiber| <http://olk.github.io/libs/fiber/doc/html/fiber/fiber_
> mgmt/fiber.html#class_fiber> constructor are consumed by the framework.
> If you need to know which exception was thrown, use |future<>| <
> http://olk.github.io/libs/fiber/doc/html/fiber/synchronization/futures/
> future.html#class_future> and |packaged_task<>| <
> http://olk.github.io/libs/fiber/doc/html/fiber/synchronization/futures/
> packaged_task.html#class_packaged_task>. "

OK

>  Which exception is thrown when the Error Conditions:resource_deadlock_
>>> would_occurand
>>> invalid_argument are signaled?
>>>
>>>  I use BOOST_ASSERT instead of exception
>>
> Where is this documented?

it's not documented - I'll add some notes

> What do you think about adding them to the class atributes also?

it would be possible - I've not thought on this variant

> This doesn't respond to my question. Why the change of thread_affinity
> need to be changed by the thread itself and by the fiber owner?

- fiber owner might deside on some point that a specia fiber is save to be
migrated to another thread
- it  is the fiber itself (code running in the fiber) which can modify its
thread-affinity

-> the user (code) can decide when I fiber is safe to be selected for
migrating between threads

> Could you explain why it is dangerous and how it is more dangerous than
> using the safe bool idiom?

struct X {
  operator bool() {}
};

struct Y {
  operator bool() {}
};

X x; Y y;

if ( x == y) // does compile

> How the user selects the thread to which the scheduling algorithm is
> applied? the current thread?
> If yes, what about adding the function on a this_thread namespace?
>
>     boost::fibers::this_thread::set_scheduling_algorithm(  &  mfs);

maybe - I would not add too many nested namespaces

> "Note:
>
>    |set_scheduling_algorithm()| does /not/ take ownership of the passed
>    |algorithm*|: *Boost.Fiber* does not claim responsibility for the
>    lifespan of the referenced |scheduler| object. The caller must
>    eventually destroy the passed |scheduler|, just as it must allocate
>    it in the first place. (Storing the pointer in a
>    |boost::thread_specific_ptr| is one way to ensure that the instance
>    is destroyed on thread termination.)"

there is no special trick - it is the code below which installs the default
scheduler if the user
does not call set_scheduling_algorithm().

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

 Maybe schedulers need to take care of a single time_point, but the other
> classes should provide an time related interface using any clock.

OK - boost.chrono is your domain. functions accepting a
time_duration/time_point (from boost.chrono) can always be mapped/applied
to steady_clock/system_clock?

>
> I made some adaptations to boost::barrier that could also have a sens for
>> fibers.
>>
>
> OK - what are those adaptations?
>
See
> http://www.boost.org/doc/libs/1_55_0/doc/html/thread/
> synchronization.html#thread.synchronization.barriers and
> http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2013/
> n3817.html#barrier_operations

OK - that's new to me, but I don't know the use case of completion function
etc. - do you have some hints?

> Ok. Glad to see that you have tried it.

maybe in another library the combination of those two kinds of sync.
primitives will succeed

> Could you explain the rationale?
>
>   Element queue::value_pop();
>
> can be used with no default constructible types
>
> while
>
>   queue_op_status queue::pop(Element &);
>
> not.
>

yes - you are right. I was more focused on a symmetry of the interface
(returning queue_op_status)