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

Subject: Re: [boost] Boost.Fiber review January 6-15
From: Vicente J. Botet Escriba (vicente.botet_at_[hidden])
Date: 2014-01-09 17:10:54

Le 09/01/14 20:28, Oliver Kowalke a écrit :
> 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
>
I don't see the problem with explicit conversion

struct X {
   explicit operator bool() {}
};

struct Y {
   explicit operator bool() {}
};

>> 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
It has the advantage to be clear. As you can see I was confused to what 
the function was applying.
>
>
>> "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();
>> }
>>
Is there an example showing this on the repository?
Can the scheduler be shared between threads?
>
>   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?
No mapping can be done between clocks and there is no need to do this 
mapping. This is how standard chrono libray was designed. Please take a 
look at the following implementation

         template <class Clock, class Duration>
         bool try_lock_until(const chrono::time_point<Clock, Duration>& t)
         {
           using namespace chrono;
           system_clock::time_point     s_now = system_clock::now();
           typename Clock::time_point  c_now = Clock::now();
           return try_lock_until(s_now + ceil<nanoseconds>(t - c_now));
         }
         template <class Duration>
         bool try_lock_until(const 
chrono::time_point<chrono::system_clock, Duration>& t)
         {
           using namespace chrono;
           typedef time_point<system_clock, nanoseconds> nano_sys_tmpt;
           return 
try_lock_until(nano_sys_tmpt(ceil<nanoseconds>(t.time_since_epoch())));
         }

Note that only the try_lock_until(const 
chrono::time_point<chrono::system_clock, nanoseconds>& ) function needs 
to be virtual.
>
>> 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?
 From the pointed proposal
"

        A Note on Completion Functions and Templates

The proposed barrier takes an optional completion function, which may 
either return void or size_t. A barrier may thus do one of three things 
after all threads have called |count_down_and_wait()|:

  * Reset itself automatically (if given no completion function.)
  * Invoke the completion function and then reset itself automatically
    (if given a function returning void).
  * Invoke the completion function and use the return value to reset
    itself (if given a function returning size_t)."

As you can see this parameter is most of the time related to how to 
reset the barrier counter.

>
>
>> 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)
>
Great.

Vcente