Re: [boost] [threadpool] new version - with rescheduling ofcurrenttask

Subject: Re: [boost] [threadpool] new version - with rescheduling ofcurrenttask
From: vicente.botet (vicente.botet_at_[hidden])
Date: 2009-02-28 04:43:10

----- Original Message ----- 
From: <k-oli_at_[hidden]>
To: <boost_at_[hidden]>
Sent: Saturday, February 28, 2009 8:28 AM
Subject: Re: [boost] [threadpool] new version - with rescheduling ofcurrenttask

> 
> Hello Vicente,
> 
> Am Saturday 28 February 2009 01:24:43 schrieb vicente.botet:
>> BTW, the doc html is not included
> 
> sorry - I'll add this soon

Thanks. 

>> > I was expecting that fro a long time. I was tried to do that, but the
>> > time goes on.  Thanks a lot Oliver, I will take a deep look in.
>> >
>> >> I've also added two function in order to wait for multiple tasks (it
>> >> uses Anthonies future lib wait_for_any()/wait_for_all() ).
>> >
>> > So you don't need any more to provide get_future, isn't it?
> 
> Yes - the future should be an implementation detail for the task object.

If I understand then we will have two function win the same name in different namespaces. Does ADL found the good one,i.e. if the user see boths overloadings, will wait_for_all(f1, f2) call the the future overloading and wait_for_all(t1, t2) calls the task overloading or do we need to prefix with the namespace?    
 
> I believe the lib should provide a this_task::get_thread_pool() too (as you 
> suggested).
> I've also some concerns about this_task::reschedule_until( function< bool() > 
> const&) because the passed function object could block in bool operator()() 
> (->for instance blocking in locking a mutex etc.).
> Maybe reschedule_until() should only accept futures?! Or schould I remove this 
> function because pool::submit() called inside of a worker-thread would 
> reschedule too.

Well the question is, what happens if the condition don't retuns true? The task doing this will be blocked, but only this task. The worker thread will continue to schedule other tasks, of course testing each time this condition. It seems to me a good compromise.

Oliver, I think that one of the use cases is to be able to implement with the public interface

namespace this_task {
    void sleep_until(t);    
}

namespace this_task {
    namespace detail {
        struct time_reached {
            system_time& abs_time_;
            time_reached(system_time& abs_time) : abs_time_(abs_time) {}
            bool operator()() {
                return boost::get_system_time() >= abs_time_;
            }
        };
    }

    void sleep_until(system_time& abs_time) {
            detail::time_reached t(abs_time);
            this_task::reschedule_until(t);
    }

}

This sounds good, but do we want the code know if it is running on a thread_pool worker thread? In my opinion the code must be transparent, so in case the thread is not a worker we need just to sleep
  
    void sleep_until(system_time& abs_time) {
        if (this_task::pressent()) {
            detail::time_reached t(abs_time);
            this_task::reschedule_until(t);
        } else {
            this_thread::sleep(abs_time);
        }
    }

So the function present() or any better name should be added.

bool this_task::pressent() {
    return (tp::detail::pool_base::tss_worker_.get()!0);
}

BTW, the interface could be replaced 

    void reschedule_until( function< bool() > const& cond)

by this one that seems more efficient to me.

    template <typename Condition>
    void reschedule_until( Condition const cond);

I have not yet tried to modify Boost.Interthreads to use the new interfaces. I'll do as soos as possible, to avoid divergencies.

I think the library starts to be ready for review. 

Good work Oliver,
Vicente