Re: [boost] [context] mini-review comments

Subject: Re: [boost] [context] mini-review comments
From: Oliver Kowalke (oliver.kowalke_at_[hidden])
Date: 2012-01-13 02:14:51

Hello Jeff,

thx for your review 

> How do you store the function and function arguments passed to the 
> context constructor?

the function arguments are bound with the function to a functor using boost::bind(). This functor is stored inside context_object class (derived from context_base (both in sub-namespace detail).

> I couldn't find it in the documentation, and whatever the answer, seems
> like something that should be mentioned.

Hmm - it is an implementation detail. Should I document it in the 'Rational' section?

> "The maximum number of arguments of *context-function* is defined by
> BOOST_CONTEXT_ARITY (default is 10) and might be changed."
> Hopefully you mean, more specifically, that BOOST_CONTEXT_ARITY might be
> *increased*.

that's the intention

> Also, consider using variadic templates if available?

of course

> Great examples! Would it be possible to also show the expected output,
> just so the reader can verify that he or she understands what should 
> be happening?

yes

> Given that void pointers are used to communicate between contexts,
> I almost would've expected the context constructor to simply take
> a void (*)( void*) argument and context::start to take a void* 
> argument which would be passed to the function pointer.
> Can you elaborate on the rationale for the low-level interface for
> communicating between contexts while still providing a relatively 
> high level interface for specifying the context function?

With context::start you enter the context-function. The arguemnts for the context-fn are taken by the ctor of context.
context::resume and context::suspend have a void* arg because it is used to transfer data between context jumps.

context ctx;
void f( int i) {
 ...
 char * x = "abc";
 void * vp = ctx.suspend( x);
 double * d = static_cast< double >( vp);
 ...
}
ctx = context( f, 1);
void * vp = ctx.start();
char * x = static_cast< char * >( vp);
double d = 2.01;
ctx.resume( & d);

the cotnext-fn 'f()' is passed to contexts ctor with the integer arg '1'.
ctx.start() starts the context == enter f() with argument i == 1.
ctx.suspend() suspends execution of context-fn f() so we return from ctx.start() and transfers x == 'abc' as return arg of ctx.start().
ctx.resume() re-enters context-fn f() and the return value of ctx.suspend() is a pointer to the double withvalue 2.01.

> The documentation for the stack allocator concept doesn't completely
> define the semantics of the size parameter of
> stack_allocator::allocate/deallocate. I presume it is the stack size in
> bytes, but that should be explicit in the documentation. And I presume
> allocate is only required to return a pointer to *at least* size bytes of
> free memory.

yes

> Also, you don't mention alignment requirements; what are
> they?

the stack will be aligned automatically by boost.context - the user has not to worry about it.

> I'm somewhat surprised default_stacksize() always returns 256*1024. I
> think
> it might be safer to make this implementation-defined, but some minimum
> size (as with everything else in C++...).

which value should be choosen for default_stacksize()? The operating system doesn't define it (but minimum/maximum stacksize is defined) - it is a heuristic value.

> Just a nitpick on member variable names.
> boost_fcontext_stack::ss_base/ss_limit: what does "ss" mean?

names base and limit are inspired by MS Windows Fiber API.

> In "Rationale", you give "[o]ther low level APIs and their caveats"; I'm
> guessing these are, specifically, alternatives to boost_fcontext_t but 
> are *deficient* in some respect. It might clear things up if you make 
> that explicit, and further group "setjump()/longjmp()", "ucontext_t",
> and "Windows fibers" into a common heading.

OK

regards,
Oliver
-- 
Empfehlen Sie GMX DSL Ihren Freunden und Bekannten und wir
belohnen Sie mit bis zu 50,- Euro! https://freundschaftswerbung.gmx.de