Hi Oliver, Vicente, all, Here is my uneducated review of the Context library. I would like to see the library Accepted. However, I do not have the expertise to pore over assembly instructions, nor do I know whether this is the right approach. I would probably be just as happy with a slower safe cross-platform library built on OS support, as with the fast (dangerous?) assembly version. I hope that people who know better, like Giovanni and Holger, will continue to help make the library more robust.
- What is your evaluation of the design?
From a user's perspective, it seems fairly clear how to use the library. I thought I had understood from discussion that the class is moveable, but this didn't work for me. I ended up having to wrap contexts up in shared_ptrs in order to get my example to work (attached). I am confident that higher-level libraries will take away the ugliness that arises from both coroutines in my example having to have access to the context for the other routine. I admire the way Oliver split and then split his library again, to make this a very manageable library for review.
- What is your evaluation of the implementation?
I only glanced at it.
- What is your evaluation of the documentation?
The documentation is quite spare and completely lacks examples. It needs copy-editing (I can help there). It also seems to be out of date. For example, it states: "To get the current user-level context boost::context::current() has to be called" but I couldn't find this function at all; it seems to have been replaced with the default-constructed context<>?
- What is your evaluation of the potential usefulness of the library?
It would be great to have teleportation (or is it time travel? ;-) in C++. I expect this will unlock many interesting paradigms in addition to coroutines & generators. I ran into a problem recently that I thought would be nicely solved if only I had coroutines. A parallelization framework had a callback when it needed more tasks to send out, and the routine that produced tasks was a hairy loop-of-loops. Neither could have its control inverted without making a big mess. I've attached an extreme simplification of the problem (without the callback part that made it impossible to invert control).
- Did you try to use the library? With what compiler? Did you have any problems?
Yes. I used Ubuntu 9.10 (in a VM), gcc version 4.4.1 (Ubuntu 4.4.1-4ubuntu8). Since I used the review version from the vault and didn't do anything special, I think this means I used the ucontext version (how do I try fcontext? docs don't seem to explain how to choose.) I had one problem which turned out to be a misunderstanding: I tried to construct a context with a next-context and then call it multiple times, but this crashed. In retrospect this isn't any better than a regular function call! and doesn't make much sense. But it would be good for the documentation to make it clear that a context is dead if the function returns.
- How much effort did you put into your evaluation? A glance? A quick reading? In-depth study?
I spent a couple of hours reading docs and discussion, a few hours getting my example running, and an hour or two on writing this review.
- Are you knowledgeable about the problem domain?
Color me interested but no expert. Cheers, Gordon
On Mar 27, 2011, at 8:48 AM, Gordon Woodhull wrote:
I ended up having to wrap contexts up in shared_ptrs in order to get my example to work
Of course, looking at the examples, I realize I didn't actually have to do this, a simple forward declaration would have done the trick. It does look like Coroutine's yield() nicely avoids the problem of each function having to know the other's context.
Am 27.03.2011 15:03, schrieb Gordon Woodhull:
On Mar 27, 2011, at 8:48 AM, Gordon Woodhull wrote:
I ended up having to wrap contexts up in shared_ptrs in order to get my example to work Of course, looking at the examples, I realize I didn't actually have to do this, a simple forward declaration would have done the trick.
It does look like Coroutine's yield() nicely avoids the problem of each function having to know the other's context.
yes - boost.context is only a building block. boost.tasklet (as a higher level abstraction) provides such a facility too -> boost::this_tasklet::yield() suspends the current execution context. Oliver
However, I do not have the expertise to pore over assembly instructions, nor do I know whether this is the right approach. Assembler is the only way to access and set stack pointer, instruction
I would probably be just as happy with a slower safe cross-platform library built on OS support, as with the fast (dangerous?) assembly version. On UNIX ucontext_t is deprecated by the new POSIX standard (and might not be available in the next releases of C-libs). This has its reason in
Hello Gordon, thank you for your review - getting input/opinion of other people helps to shape the lib. Am 27.03.2011 14:48, schrieb Gordon Woodhull: pointer and the other registers. the fact that the interface of makecontext() function was designed before C99. It expects a function-pointer of void(*)() and accepts ellipses/var args for the arguments accepted by the function to be called by ucontext_t. I'm not the big C expert but I believe following cast is not legal (even if some compilers accept it): void my_fn( int, char *); (void(*)())my_fn; // not legal But the signature of ucontext-function makecontext( ucontext_t *, void(*)(), int, ...) requires this cast. Even worse - on x86_64/Linux the glibc the usage of non integer values as argument for the var-args is undefined. example: void my_fn( X * x); // to be called by ucontext X * x = new X("abc"); ucontext_t ctx; makecontext( & ctx, (void(*)())(my_fn), 1, x); // not reliable; undefined behaviour In the example above it might work (if the high bits of the address of x are zero) but in some cases makecontext() truncates the high bit of x pointer :/ => segfault Therefore I prefer the fcontext version over ucontext because it hasn't this problem. On Windows I suggest using the Windows Fiber version because as Holger mentioned I've to correct the exception handling.
I hope that people who know better, like Giovanni and Holger, will continue to help make the library more robust.
I hope it too :)
I thought I had understood from discussion that the class is moveable, but this didn't work for me. I ended up having to wrap contexts up in shared_ptrs in order to get my example to work (attached).
It was a misunderstanding the in review version boost::context is noncopyable - but it was in a previous version moveable (pimpl idiom). I currently modify the lib so that boost::context supports move operations.
The documentation is quite spare and completely lacks examples. It needs copy-editing (I can help there).
that would be really nice regards, Oliver
Hi Oliver, On Mar 27, 2011, at 9:22 AM, Oliver Kowalke wrote:
Gordon wrote:
I would probably be just as happy with a slower safe cross-platform library built on OS support, as with the fast (dangerous?) assembly version. On UNIX ucontext_t is deprecated by the new POSIX standard (and might not be available in the next releases of C-libs). This has its reason in the fact that the interface of makecontext() function was designed before C99. It expects a function-pointer of void(*)() and accepts ellipses/var args for the arguments accepted by the function to be called by ucontext_t.
Do you mean that the functionality might go away, or will be replaced by a new API?
I'm not the big C expert but I believe following cast is not legal (even if some compilers accept it):
void my_fn( int, char *); (void(*)())my_fn; // not legal
But the signature of ucontext-function makecontext( ucontext_t *, void(*)(), int, ...) requires this cast.
Even worse - on x86_64/Linux the glibc the usage of non integer values as argument for the var-args is undefined. example:
void my_fn( X * x); // to be called by ucontext
X * x = new X("abc"); ucontext_t ctx; makecontext( & ctx, (void(*)())(my_fn), 1, x); // not reliable; undefined behaviour
In the example above it might work (if the high bits of the address of x are zero) but in some cases makecontext() truncates the high bit of x pointer :/ => segfault
Therefore I prefer the fcontext version over ucontext because it hasn't this problem.
Hmm. Not really following you here. My understanding was that the proposed Boost.Context only accepts void f(void*) at this time - are you describing functionality that is not exposed because you have to stay compatible with ucontext? I hope you aren't saying that the ucontext version is unreliable!
On Windows I suggest using the Windows Fiber version because as Holger mentioned I've to correct the exception handling.
My preference, for all platforms, as someone who considers all this assembly stack manipulation to be (not my kind of) black magic, would be to have both a 100% safe OS implementation and a fast hand-rolled implementation available via a #define BTW, I'm still not clear how to choose which implementation I'm using. I have the review version.
I thought I had understood from discussion that the class is moveable, but this didn't work for me. I ended up having to wrap contexts up in shared_ptrs in order to get my example to work (attached).
It was a misunderstanding the in review version boost::context is noncopyable - but it was in a previous version moveable (pimpl idiom). I currently modify the lib so that boost::context supports move operations.
Used to have the feature, will have it again... got it!
The documentation is quite spare and completely lacks examples. It needs copy-editing (I can help there).
that would be really nice
Will do. Cheers, Gordon
Hello Gordon, Am 28.03.2011 07:28, schrieb Gordon Woodhull:
Hi Oliver,
On Mar 27, 2011, at 9:22 AM, Oliver Kowalke wrote:
Gordon wrote:
I would probably be just as happy with a slower safe cross-platform library built on OS support, as with the fast (dangerous?) assembly version. On UNIX ucontext_t is deprecated by the new POSIX standard (and might not be available in the next releases of C-libs). This has its reason in the fact that the interface of makecontext() function was designed before C99. It expects a function-pointer of void(*)() and accepts ellipses/var args for the arguments accepted by the function to be called by ucontext_t. Do you mean that the functionality might go away, or will be replaced by a new API?
yes - it is dprecated by the new POSIX standard. For instance glibc on ARM doesn't provide ucontext anymore. that was the reason for implementing the functionality in assembler => fcontext (without the limitations of ucontext).
I'm not the big C expert but I believe following cast is not legal (even if some compilers accept it):
void my_fn( int, char *); (void(*)())my_fn; // not legal
But the signature of ucontext-function makecontext( ucontext_t *, void(*)(), int, ...) requires this cast.
Even worse - on x86_64/Linux the glibc the usage of non integer values as argument for the var-args is undefined. example:
void my_fn( X * x); // to be called by ucontext
X * x = new X("abc"); ucontext_t ctx; makecontext(& ctx, (void(*)())(my_fn), 1, x); // not reliable; undefined behaviour
In the example above it might work (if the high bits of the address of x are zero) but in some cases makecontext() truncates the high bit of x pointer :/ => segfault
Therefore I prefer the fcontext version over ucontext because it hasn't this problem. Hmm. Not really following you here. My understanding was that the proposed Boost.Context only accepts void f(void*) at this time - are you describing functionality that is not exposed because you have to stay compatible with ucontext? I hope you aren't saying that the ucontext version is unreliable!
ucontext is wrapped by boost::context - and in the new version of boost.context ucontext will be used only as fallback for platforms where fcontext implementation isn't available. And yes - ucontext is buggy - from its usage (invalid function pointer cast) and behaviour (the 64bit-pointer truncation of arguments with glibc/x86_64).
On Windows I suggest using the Windows Fiber version because as Holger mentioned I've to correct the exception handling. My preference, for all platforms, as someone who considers all this assembly stack manipulation to be (not my kind of) black magic, would be to have both a 100% safe OS implementation and a fast hand-rolled implementation available via a #define
BTW, I'm still not clear how to choose which implementation I'm using. I have the review version.
The documentation contains a chapter 'Configuration', it describes the required bjam command-line arguments for the different platforms: x86/32bit: bjam toolset0gcc architecture=x86 instruction-set=i686 address-model=64 context-impl=asm I know that's a lot of args but it is triggered by some limitations of the current version of boost.build (Vladimir addresses this issue already). so long, Oliver
Hi Oliver, On Mar 28, 2011, at 4:03 AM, Oliver Kowalke wrote:
yes - it is dprecated by the new POSIX standard. For instance glibc on ARM doesn't provide ucontext anymore. that was the reason for implementing the functionality in assembler => fcontext (without the limitations of ucontext).
I see. That's a shame. I agree with Artyom that it makes sense for you to create a C API as well. I would have thought C programmers use coroutines too much for POSIX to get away with this. User-level threads are pretty common too, aren't they?
ucontext is wrapped by boost::context - and in the new version of boost.context ucontext will be used only as fallback for platforms where fcontext implementation isn't available. And yes - ucontext is buggy - from its usage (invalid function pointer cast) and behaviour (the 64bit-pointer truncation of arguments with glibc/x86_64).
Well, I'm glad you're "keeping a foot in the door". I hope ucontext will get reincarnated. Seems like they could make it safe just by getting rid of the varargs, if that was the concern. void f(void*) is really all you need.
The documentation contains a chapter 'Configuration', it describes the required bjam command-line arguments for the different platforms:
x86/32bit: bjam toolset0gcc architecture=x86 instruction-set=i686 address-model=64 context-impl=asm
OK, sorry for confusion. I got these settings but for some reason wasn't sure they were working; I am probably successfully using fcontext then. Thanks! Gordon
participants (2)
-
Gordon Woodhull -
Oliver Kowalke