[context review] Review - alternative version available
Hi, I've uploaded an alternative version to boost vault (boost.context-alternative_version.zip : http://tinyurl.com/6fpnkro). The documentation is available at http://ok73.ok.funpic.de/alternative/boost/libs/context/doc/html/. changes: - fixed bug for x86_64/sysv/elf assembler file: missing assembler directive .type added - as suggested by Artyom and Vicente: - assembler implementation and native implementation of context switching functions compiled and applied as template arg to context<> -> context<> == context< protected_stack, asm_impl > on UNIX, context< protected_stack, native_impl > on Windows - for Windows assembler implementation is disabled Oliver
Message du 22/03/11 21:09 De : "Oliver Kowalke" A : boost-users@lists.boost.org, boost@lists.boost.org Copie à : Objet : [boost] [context review] Review - alternative version available
Hi,
I've uploaded an alternative version to boost vault (boost.context-alternative_version.zip : http://tinyurl.com/6fpnkro). The documentation is available at http://ok73.ok.funpic.de/alternative/boost/libs/context/doc/html/.
changes:
- fixed bug for x86_64/sysv/elf assembler file: missing assembler directive .type added - as suggested by Artyom and Vicente: - assembler implementation and native implementation of context switching functions compiled and applied as template arg to context<> -> context<> == context< protected_stack, asm_impl > on UNIX, context< protected_stack, native_impl > on Windows - for Windows assembler implementation is disabled
Oliver, please it would be simpler if we review only one library. You could post here whatever you want, patches, explanations, but please don't request the reviewers to review another package. Could you elaborate more on the asm_impl point, I think I've missed something? Please post here whatever do you want to change. Best, Vicente
Am 22.03.2011 21:20, schrieb Vicente BOTET:
Oliver, please it would be simpler if we review only one library. You could post here whatever you want, patches, explanations, but please don't request the reviewers to review another package.
ok
Could you elaborate more on the asm_impl point, I think I've missed something? Please post here whatever do you want to change.
Artyom has requested in its last postings that boost.context should compile the implementations for fcontext (using assembler) as well ucontext/Win Fiber (provided by the operating system) instead select one at compile-time with bjam options. The user can now decide which impl should be choosen by selecting the corresponding class: context< stack, asm_impl > : uses fcontext/assembler if available (Windows is currently disabled) context< stack, native_impl > : uses ucontext from C-lib on UNIX (if implemented) and Windows Fibers on Windows I believe Artyom has right with his concerns about the ABI issues (binary compatiblity) - I would probably prefer the alternative solution. Because I got little feedback in the last year I'd like to use this review to get some comments for the alternative version. I remember you already ask for such a solution some weeks ago. regards, Oliver
Message du 22/03/11 21:50 De : "Oliver Kowalke" A : boost@lists.boost.org Copie à : Objet : Re: [boost] [context review] Review - alternative version available
Am 22.03.2011 21:20, schrieb Vicente BOTET:
Oliver, please it would be simpler if we review only one library. You could post here whatever you want, patches, explanations, but please don't request the reviewers to review another package.
ok
Could you elaborate more on the asm_impl point, I think I've missed something? Please post here whatever do you want to change.
Artyom has requested in its last postings that boost.context should compile the implementations for fcontext (using assembler) as well ucontext/Win Fiber (provided by the operating system) instead select one at compile-time with bjam options.
The user can now decide which impl should be choosen by selecting the corresponding class:
context< stack, asm_impl > : uses fcontext/assembler if available (Windows is currently disabled) context< stack, native_impl > : uses ucontext from C-lib on UNIX (if implemented) and Windows Fibers on Windows
I have a question with this design. Which will be the parameter used by Boost.Fiber, Boost.Taslet? Should them also provide this parameter? I really think this is not a good option. The good solution for me will be that the library doesn't let the user choose the implementation for a given platform, so ABI problem are avoided If I remember the problem comes from the fact that the assembler used can not be compiled conditionally as not included in a C++ file. If you want to preserve this file structure you will need a build system that let you know what are the best implementation you can provide on a specific platform.
I believe Artyom has right with his concerns about the ABI issues (binary compatiblity) - I would probably prefer the alternative solution. Because I got little feedback in the last year I'd like to use this review to get some comments for the alternative version.
I remember you already ask for such a solution some weeks ago.
Not exactly. What I was requesting is what you provide today with the context been non-copyable and non-movable. Maybe you are thinking to the fact I requested you to make a uniform C interface for fcontext and implement it with ucontext or fibers when the asm implementation is not available. This is in line with the request of Arytom to provide a C-interface. Best, Vicente
The user can now decide which impl should be choosen by selecting the corresponding class:
context< stack, asm_impl> : uses fcontext/assembler if available (Windows is currently disabled) context< stack, native_impl> : uses ucontext from C-lib on UNIX (if implemented) and Windows Fibers on Windows I have a question with this design. Which will be the parameter used by Boost.Fiber, Boost.Taslet? Should them also provide this parameter? good question - because I'm currently focused on the boost.context review I didn't spend much time of the influences of the changed design -
Am 22.03.2011 22:26, schrieb Vicente BOTET: probably as template argument.
I really think this is not a good option. The good solution for me will be that the library doesn't let the user choose the implementation for a given platform, so ABI problem are avoided If I remember the problem comes from the fact that the assembler used can not be compiled conditionally as not included in a C++ file.
The problem is that the user can't choose between a boost::context<> version which can deal with UNIX signals (as ucontext functions do) or a fast assembler implementation (fcontext) if UNIX signals are out of scope/irrelevant /because handled in another place/thread - what ever).
I believe Artyom has right with his concerns about the ABI issues (binary compatiblity) - I would probably prefer the alternative solution. Because I got little feedback in the last year I'd like to use this review to get some comments for the alternative version.
I remember you already ask for such a solution some weeks ago. Not exactly. What I was requesting is what you provide today with the context been non-copyable and non-movable. OK - that is already done :^)
Maybe you are thinking to the fact I requested you to make a uniform C interface for fcontext and implement it with ucontext or fibers when the asm implementation is not available. This is in line with the request of Arytom to provide a C-interface.
if it is desired I'll do it - but how does it fit into boost (which is C++ centric/related)? regards, Oliver
Message du 22/03/11 22:44 De : "Oliver Kowalke" A : boost@lists.boost.org Copie à : Objet : Re: [boost] [context review] Review - alternative version available
The user can now decide which impl should be choosen by selecting the corresponding class:
context< stack, asm_impl> : uses fcontext/assembler if available (Windows is currently disabled) context< stack, native_impl> : uses ucontext from C-lib on UNIX (if implemented) and Windows Fibers on Windows I have a question with this design. Which will be the parameter used by Boost.Fiber, Boost.Taslet? Should them also provide this parameter? good question - because I'm currently focused on the boost.context review I didn't spend much time of the influences of the changed design -
Am 22.03.2011 22:26, schrieb Vicente BOTET: probably as template argument.
I really think this is not a good option. The good solution for me will be that the library doesn't let the user choose the implementation for a given platform, so ABI problem are avoided If I remember the problem comes from the fact that the assembler used can not be compiled conditionally as not included in a C++ file.
The problem is that the user can't choose between a boost::context<> version which can deal with UNIX signals (as ucontext functions do) or a fast assembler implementation (fcontext) if UNIX signals are out of scope/irrelevant /because handled in another place/thread - what ever).
Well, I believe that unix signals should be handled in a single place, but I'm not an expert in this domain. I would like to see a good rationale explaining the advantages of letting Boost.Context manage them. In any case the name asm_impl and native_impl are not adapted. I let you make some better suggestions than the current ones. Best, Vicente
The problem is that the user can't choose between a boost::context<> version which can deal with UNIX signals (as ucontext functions do) or a fast assembler implementation (fcontext) if UNIX signals are out of scope/irrelevant /because handled in another place/thread - what ever).
Well, I believe that unix signals should be handled in a single place, but I'm not an expert in this domain. I would like to see a good rationale explaining the advantages of letting Boost.Context manage them.
The UNIX signals are not handled/managed by boost.context. In the case of ucontext (native_impl) the thread which runs boost.context can unblock and handle UNIX signals. For instance a platform which doesn't support kernel threads but has UNIX signal handling must handle signals in the same thread. In the other case (fcontext == asm_impl) the thread must block all UNIX signals.
In any case the name asm_impl and native_impl are not adapted. I let you > make some better suggestions than the current ones.
Well you know finding decriptive names for classes is not so easy (at least for me) - do you have a suggestion? regards, Oliver -- Empfehlen Sie GMX DSL Ihren Freunden und Bekannten und wir belohnen Sie mit bis zu 50,- Euro! https://freundschaftswerbung.gmx.de
Hello Vicente,
I would like to see a good rationale explaining the advantages of letting Boost.Context manage them. In any case the name asm_impl and native_impl are not adapted. I let you make some better suggestions than the current ones.
What about dropping context< stack, impl > in favour of fast_context< stack > and native_context< stack > (nastive_context<> doesn't describe it well - do you have a better idea?). The user has the opertunity to choos which context he wants to use. regards, Oliver -- GMX DSL Doppel-Flat ab 19,99 Euro/mtl.! Jetzt mit gratis Handy-Flat! http://portal.gmx.net/de/go/dsl
Message du 23/03/11 14:51 De : "Oliver Kowalke" A : boost@lists.boost.org Copie à : Objet : Re: [boost] [context review] Review - alternative version available
Hello Vicente,
I would like to see a good rationale explaining the advantages of letting Boost.Context manage them. In any case the name asm_impl and native_impl are not adapted. I let you make some better suggestions than the current ones.
What about dropping context< stack, impl > in favour of fast_context< stack > and native_context< stack > (nastive_context<> doesn't describe it well - do you have a better idea?). The user has the opertunity to choos which context he wants to use.
Hi, I'm still looking for the use case. What could make a user that has the possibility to get a fast context switch 13x faster to prefers to use the low context provided using ucontext? Best, Vicente
Am 23.03.2011 17:35, schrieb Vicente BOTET:
Message du 23/03/11 14:51 De : "Oliver Kowalke" A : boost@lists.boost.org Copie à : Objet : Re: [boost] [context review] Review - alternative version available
Hello Vicente,
I would like to see a good rationale explaining the advantages of letting Boost.Context manage them. In any case the name asm_impl and native_impl are not adapted. I let you make some better suggestions than the current ones. What about dropping context< stack, impl> in favour of fast_context< stack> and native_context< stack> (nastive_context<> doesn't describe it well - do you have a better idea?). The user has the opertunity to choos which context he wants to use. Hi,
I'm still looking for the use case. What could make a user that has the possibility to get a fast context switch 13x faster to prefers to use the low context provided using ucontext?
You suggest that context<> should use fcontext if available and native OS facility otherwise?! If the other people agree I'll implement it. Maybe I should remove the stack template argument so that context onnly supports protected_stack (for security reasons -> overwriting memoryof the app if stacksize is too small)? best regards, Oliver
Message du 23/03/11 17:47 De : "Oliver Kowalke" A : boost@lists.boost.org Copie à : Objet : Re: [boost] [context review] Review - alternative version available
Am 23.03.2011 17:35, schrieb Vicente BOTET:
Message du 23/03/11 14:51 De : "Oliver Kowalke" A : boost@lists.boost.org Copie à : Objet : Re: [boost] [context review] Review - alternative version available
Hello Vicente,
I would like to see a good rationale explaining the advantages of letting Boost.Context manage them. In any case the name asm_impl and native_impl are not adapted. I let you make some better suggestions than the current ones. What about dropping context< stack, impl> in favour of fast_context< stack> and native_context< stack> (nastive_context<> doesn't describe it well - do you have a better idea?). The user has the opertunity to choos which context he wants to use. Hi,
I'm still looking for the use case. What could make a user that has the possibility to get a fast context switch 13x faster to prefers to use the low context provided using ucontext?
You suggest that context<> should use fcontext if available and native OS facility otherwise?! If the other people agree I'll implement it. Maybe I should remove the stack template argument so that context only supports protected_stack (for security reasons -> overwriting memory of the app if stacksize is too small)?
I remember that protected_stack made use of mmap. I will prefer to been able to don't depend on mmap for this purpose and been able to use static allocation. I don't know if this is pre-optimization. Best, Vicente
I've uploaded an alternative version to boost vault (boost.context-alternative_version.zip : http://tinyurl.com/6fpnkro). The documentation is available at http://ok73.ok.funpic.de/alternative/boost/libs/context/doc/html/.
changes:
- fixed bug for x86_64/sysv/elf assembler file: missing assembler directive .type added - as suggested by Artyom and Vicente: - assembler implementation and native implementation of context switching functions compiled and applied as template arg to context<> -> context<> == context< protected_stack, asm_impl > on UNIX, context< protected_stack, native_impl > on Windows - for Windows assembler implementation is disabled
Oliver
It is much better :-) And the crash bug is gone. Artyom
participants (3)
-
Artyom -
Oliver Kowalke -
Vicente BOTET