Re: [boost] [context review] Updated vote

Subject: Re: [boost] [context review] Updated vote
From: Giovanni Piero Deretta (gpderetta_at_[hidden])
Date: 2011-03-28 10:57:32

On Mon, Mar 28, 2011 at 3:14 PM, Oliver Kowalke <oliver.kowalke_at_[hidden]> wrote:
> Hello Giovanni,
>
> thank you for your review (and of course all others) - it really helps to
> get the lib shaped (I got very view responses before the review).
>
> I've started to modify the lib (you can take a look at
> git://gitorious.org/boost-dev/boost-dev.git, branch context-dev) in order to
> address the issues raised by the review.
> If I believe all issues are addressed we can start a mini-review (or I'll
> send you and the other reviews an eMail for checking the new variant for
> discussion).
>

Re mail: yes please. I'm taking a look at the code and I see you are
doing quite a bit of changes.

I couple of initial comments:

- remember that all extern "C" functions are not really in the
boost::contexts::details namespace, so you should really add a unique
prefix to them (boost_context_* for example)

- To support stateful functors you are using std::function that imply
a certain overhead.

Why not use something like this:

assuming you have a C like make_context(context_impl*, void (*f)(void
*)), change your trampoline function to something like this:

template<class T>
struct jump {
   T& x;
   context_impl *src, *dest;
};

template<typename T>
void trampoline(void * parm) {
   jump<T> & jmp = *static_cast<jump*>(parm);
   T x = jmp.x;
   swap_context(jmp.src, jmp.dest);
   x();
}

now in context constructor:

template<class T> context(T f) {
   context_impl temp_context;
   jump<T> p (f, &m_context, &temp_context);
   make_context(my_context, &trampoline<T>, &p);
   swap_context(&temp_context, &m_context); // here be magic
}

T has now been copied in at the bottom of the stack controlled by
m_context. This requires a context switch in the constructor to copy T
immediately, but it will probably cost less than constructing an
std::funciton. Note that if you control the context_impl
implementation and the associated make/swap context, you do not really
need to invoke the trampoline function to do the copy, but you can
carefully set-up the stack as if it had been done.

A similar trick can be used to save a dynamic allocation of
context_impl (for the ucontext_ case) or to make it an opaque type.

HTH,

-- 
gpd