Re: [boost] [FSM] Review library version update
AMDG Andrey Semashev wrote:
Hello,
I've updated the Boost.FSM library in the Vault with the following changes: - Several threading issues pointed out by Steven Watanabe were fixed. - Some minor optimizations added to reduce the amount of generated code and possibly speed up compilation a little. - Exception classes are now exposed with default visibility on GCC 4.0 and later under Linux. I'm not sure if other compilers/platforms need this and have support for similar feature. - A new example "BitMachine" added. It is quite similar to same-named example of Boost.Statechart and may be used as a performance test. - The Performance section of the docs updated with some test results.
First of all I think that the elapsed time is more useful than the number of iterations per second. I know that one is just the inverse of the other but the elapsed time has a linear correlation to the amount of work being done. Second, ~half the time taken for the benchmark is the overhead of the test itself rather than the actual state machine. Try this version. #include <boost/mpl/for_each.hpp> #include <boost/mpl/range_c.hpp> //! Function Object version for MPL for_each struct invoke_sm_func { typedef void result_type; invoke_sm_func(BitFSM_t& bfsm) : fsm(&bfsm) { } template<class Index> void operator()(Index) { invoke_sm<Index::value>(*fsm); } BitFSM_t* fsm; }; //! Performance test implementation void run_performance_test(BitFSM_t& fsm) { //... // Execute performance loop for (register unsigned long i = 0, end = (unsigned long)(NO_OF_PERFORMANCE_EVENTS / NO_OF_BITS); i < end; ++i) { boost::mpl::for_each<boost::mpl::range_c<int, 0, NO_OF_BITS>
(invoke_sm_func(fsm)); } //handle the remainder typedef boost::mpl::range_c<int, 0, (NO_OF_PERFORMANCE_EVENTS % NO_OF_BITS)> tail; boost::mpl::for_each<tail>(invoke_sm_func(fsm));
//... } Instead of using MPL vector/list you can create your own MPL sequence. struct StatesList_t { struct tag {}; template<unsigned int N> struct iterator { typedef BitState<N> type; typedef iterator<N + 1> next; }; typedef iterator<0> begin; typedef iterator<(1 << NO_OF_BITS)> end; }; enum { NO_OF_STATES = (1 << NO_OF_BITS) }; In this case trying to reuse code actually generates more work both for yourself and for the compiler. In Christ, Steven Watanabe
Hello Steven, Wednesday, January 31, 2007, 10:02:24 PM, you wrote:
AMDG
Andrey Semashev wrote:
- A new example "BitMachine" added. It is quite similar to same-named example of Boost.Statechart and may be used as a performance test. - The Performance section of the docs updated with some test results.
First of all I think that the elapsed time is more useful than the number of iterations per second. I know that one is just the inverse of the other but the elapsed time has a linear correlation to the amount of work being done.
The elapsed time doesn't give you an absolute view of performance since it also depends on number of iterations. But I think I may present both figures in the docs.
Second, ~half the time taken for the benchmark is the overhead of the test itself rather than the actual state machine. Try this version.
[snip]
Instead of using MPL vector/list you can create your own MPL sequence.
[snip]
In this case trying to reuse code actually generates more work both for yourself and for the compiler.
Thanks for these hints, I guess my MPL knowledge failed me this time. I'll upload the updated version shortly. -- Best regards, Andrey mailto:andysem@mail.ru
Andrey Semashev <andysem <at> mail.ru> writes:
Hello Steven,
Wednesday, January 31, 2007, 10:02:24 PM, you wrote:
AMDG
Andrey Semashev wrote:
- A new example "BitMachine" added. It is quite similar to same-named example of Boost.Statechart and may be used as a performance test. - The Performance section of the docs updated with some test results.
First of all I think that the elapsed time is more useful than the number of iterations per second. I know that one is just the inverse of the other but the elapsed time has a linear correlation to the amount of work being done.
The elapsed time doesn't give you an absolute view of performance since it also depends on number of iterations. But I think I may present both figures in the docs.
Sorry. I wasn't clear. I meant time per iteration. I looked through state_machine.hpp again You changed "very base class" to "most base class." What you mean is ultimate base class I think. basic_state_machine::m_StatesInfo needs to be volatile. At the very least least cast it to volatile for initialization. This has nothing to do with sequence points; It has everything to do with observable behavior. line 912 template< typename EventT > static state_dispatcher< EventT > const& get_state_dispatcher() { static const state_dispatcher< EventT > dispatcher; return dispatcher; } function static is not thread safe. in pseudo-code this is { static no_initialize const state_dispatcher< EventT > dispatcher; static preinitialize bool dispatcher_initialized = false; if(!dispatcher_initialized) { dispatcher_initialized = true; dispatcher.dispatcher(); } return(dispatcher); } locking_state_machine.hpp dynamic_locker is unnecessary you can use scoped_lock this_lock(false); and this_lock.lock(); to do the same thing. Also for the comparisons of mutex addresses you should use std::less<void*> instead of operator< because operator< does not guarantee a strict weak ordering for pointers. see 5.9/2 and 20.3.3/8 In Christ, Steven Watanabe
Hello Steven, Saturday, February 3, 2007, 10:05:52 PM, you wrote:
Andrey Semashev <andysem <at> mail.ru> writes:
Hello Steven,
Wednesday, January 31, 2007, 10:02:24 PM, you wrote:
AMDG
Andrey Semashev wrote:
- A new example "BitMachine" added. It is quite similar to same-named example of Boost.Statechart and may be used as a performance test. - The Performance section of the docs updated with some test results.
First of all I think that the elapsed time is more useful than the number of iterations per second. I know that one is just the inverse of the other but the elapsed time has a linear correlation to the amount of work being done.
The elapsed time doesn't give you an absolute view of performance since it also depends on number of iterations. But I think I may present both figures in the docs.
Sorry. I wasn't clear. I meant time per iteration.
I see. Will change it.
I looked through state_machine.hpp again
You changed "very base class" to "most base class." What you mean is ultimate base class I think.
Will change the comment.
basic_state_machine::m_StatesInfo needs to be volatile. At the very least least cast it to volatile for initialization. This has nothing to do with sequence points; It has everything to do with observable behavior.
But it is volatile as it's being passed to the init_states_info function which does the initialization. It's argument type is volatile. I'd really hate to make it volatile for the rest of the machine's life, it only needs to be initialized properly.
line 912 template< typename EventT > static state_dispatcher< EventT > const& get_state_dispatcher() { static const state_dispatcher< EventT > dispatcher; return dispatcher; } function static is not thread safe. in pseudo-code this is { static no_initialize const state_dispatcher< EventT > dispatcher; static preinitialize bool dispatcher_initialized = false; if(!dispatcher_initialized) { dispatcher_initialized = true; dispatcher.dispatcher(); } return(dispatcher); }
It doesn't need to be. The state_machine class is intended to be used in single-thread context. Once you have to access the machine from multiple threads you should use locking_state_machine which synchronizes on "process" function calls which is the only way "get_state_dispatcher" gets called.
locking_state_machine.hpp dynamic_locker is unnecessary you can use scoped_lock this_lock(false); and this_lock.lock(); to do the same thing.
Actually, I can't - the lightweight mutex doesn't have this. The only thing the Boost.FSM requires from the locker class is the construction/destruction locking.
Also for the comparisons of mutex addresses you should use std::less<void*> instead of operator< because operator< does not guarantee a strict weak ordering for pointers. see 5.9/2 and 20.3.3/8
Thanks, will fix. -- Best regards, Andrey mailto:andysem@mail.ru
AMDG Andrey Semashev <andysem <at> mail.ru> writes:
But it is volatile as it's being passed to the init_states_info function which does the initialization. It's argument type is volatile.
My mistake
line 912 template< typename EventT > static state_dispatcher< EventT > const& get_state_dispatcher() { static const state_dispatcher< EventT > dispatcher; return dispatcher; } function static is not thread safe.
It doesn't need to be. The state_machine class is intended to be used in single-thread context. Once you have to access the machine from multiple threads you should use locking_state_machine which synchronizes on "process" function calls which is the only way "get_state_dispatcher" gets called.
Unfortunately, it isn't that simple. The dispatcher is shared by all state machines of the same type. Therefore even apparently unrelated machines can attempt to initialize it concurrently.
locking_state_machine.hpp dynamic_locker is unnecessary you can use scoped_lock this_lock(false); and this_lock.lock(); to do the same thing.
Actually, I can't - the lightweight mutex doesn't have this. The only thing the Boost.FSM requires from the locker class is the construction/destruction locking.
I see. In Christ, Steven Watanabe
Hello Steven, Sunday, February 4, 2007, 2:09:37 AM, you wrote:
AMDG
line 912 template< typename EventT > static state_dispatcher< EventT > const& get_state_dispatcher() { static const state_dispatcher< EventT > dispatcher; return dispatcher; } function static is not thread safe.
It doesn't need to be. The state_machine class is intended to be used in single-thread context. Once you have to access the machine from multiple threads you should use locking_state_machine which synchronizes on "process" function calls which is the only way "get_state_dispatcher" gets called.
Unfortunately, it isn't that simple. The dispatcher is shared by all state machines of the same type. Therefore even apparently unrelated machines can attempt to initialize it concurrently.
Yes, you're right here. But there's nothing really dangerous in running state_dispatcher constructor more than once concurrently. I'll see what I can do about it, just to be on the safe side. -- Best regards, Andrey mailto:andysem@mail.ru
AMDG Andrey Semashev <andysem <at> mail.ru> writes:
Yes, you're right here. But there's nothing really dangerous in running state_dispatcher constructor more than once concurrently. I'll see what I can do about it, just to be on the safe side.
This has exactly the same problem that m_StatesInfo had. It can be used before it is initialized. In Christ, Steven Watanabe
Hello Steven, Monday, February 5, 2007, 9:53:35 PM, you wrote:
AMDG
Andrey Semashev <andysem <at> mail.ru> writes:
Yes, you're right here. But there's nothing really dangerous in running state_dispatcher constructor more than once concurrently. I'll see what I can do about it, just to be on the safe side.
This has exactly the same problem that m_StatesInfo had. It can be used before it is initialized.
Yes, again you're right. I'm just not very experienced in multithreaded programming. I made the dispatcher instance a static class data member so it should be initialized even before any FSM get created (I've attached the modified file). But thinking of it some more I'm not quite sure even this would make me absolutely safe. The Standard says (3.6.2/3) that non-local objects with static storage duration (which the dispatcher and, BTW, state names are) are dynamically initialized before any function or object usage in the translation unit where the objects are defined. Doesn't that mean that there's a possibility that one day more than one thread may concurrently execute this initialization by accessing a function or object in the TU? If so, then I can hardly imagine the absolutely safe way to protect the initialization of such global objects. I wouldn't even be able to safely create a mutex fearing that it would be simultaneously initialized by several threads. Is there any way to solve this? -- Best regards, Andrey mailto:andysem@mail.ru
Probably you should put it has a static local variable inside a protected function, like this: //! The method returns a reference to the only dispatcher instance static state_dispatcher const& get() { static state_dispatcher instance; return instance; } The only problem here is I'm not sure how the compilers reacted when instanciating static variable inside template functions... Fabien 2007/2/5, Andrey Semashev <andysem@mail.ru>:
Hello Steven,
Monday, February 5, 2007, 9:53:35 PM, you wrote:
AMDG
Andrey Semashev <andysem <at> mail.ru> writes:
Yes, you're right here. But there's nothing really dangerous in running state_dispatcher constructor more than once concurrently. I'll see what I can do about it, just to be on the safe side.
This has exactly the same problem that m_StatesInfo had. It can be used before it is initialized.
Yes, again you're right. I'm just not very experienced in multithreaded programming.
I made the dispatcher instance a static class data member so it should be initialized even before any FSM get created (I've attached the modified file). But thinking of it some more I'm not quite sure even this would make me absolutely safe.
The Standard says (3.6.2/3) that non-local objects with static storage duration (which the dispatcher and, BTW, state names are) are dynamically initialized before any function or object usage in the translation unit where the objects are defined. Doesn't that mean that there's a possibility that one day more than one thread may concurrently execute this initialization by accessing a function or object in the TU? If so, then I can hardly imagine the absolutely safe way to protect the initialization of such global objects. I wouldn't even be able to safely create a mutex fearing that it would be simultaneously initialized by several threads. Is there any way to solve this?
-- Best regards, Andrey mailto:andysem@mail.ru _______________________________________________ Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Hello Fabien, Monday, February 5, 2007, 10:47:37 PM, you wrote:
Probably you should put it has a static local variable inside a protected function, like this:
//! The method returns a reference to the only dispatcher instance
static state_dispatcher const& get() { static state_dispatcher instance; return instance; }
The only problem here is I'm not sure how the compilers reacted when instanciating static variable inside template functions...
That's exactly what it was before. The problem is that it's not thread-safe. -- Best regards, Andrey mailto:andysem@mail.ru
Fabien Niñoles wrote:
Probably you should put it has a static local variable inside a protected function, like this:
//! The method returns a reference to the only dispatcher instance
static state_dispatcher const& get() { static state_dispatcher instance; return instance; }
In a multithreaded app this might work on some platforms, on others it doesn't: Quote from http://msdn2.microsoft.com/en-us/library/s1sb61xd.aspx: "Assigning to a static local variable is not thread safe and is not recommended as a programming practice." Although this isn't very concise wording (IIRC, the standard says that it should work for literals assigned to PODs) it seems clear that it won't work for non-PODs... Regards, -- Andreas Huber When replying by private email, please remove the words spam and trap from the address shown in the header.
2007/2/5, Andreas Huber <ahd6974-spamgroupstrap@yahoo.com>:
Fabien Niñoles wrote:
Probably you should put it has a static local variable inside a protected function, like this:
//! The method returns a reference to the only dispatcher instance
static state_dispatcher const& get() { static state_dispatcher instance; return instance; }
In a multithreaded app this might work on some platforms, on others it doesn't:
Quote from http://msdn2.microsoft.com/en-us/library/s1sb61xd.aspx: "Assigning to a static local variable is not thread safe and is not recommended as a programming practice."
Although this isn't very concise wording (IIRC, the standard says that it should work for literals assigned to PODs) it seems clear that it won't work for non-PODs...
For assignation, sure, but I don't see any in my code... However, if that's bother, I think then only something like Loki Singleton pattern will do. You then need a double lock for accessing it correctly, something like this: static state_dispatcher const& get() { static state_dispatcher* pInstance; scoped_lock first_lock(m_mutex1) if (!pInstance) { scoped_lock second_lock(m_mutex2); if (!pInstance) { pInstance = new state_dispatcher; } } return *pInstance; } The problem is now one of performance, which Loki handle by making the scoped_lock class a template class policy.
Fabien Niñoles wrote:
In a multithreaded app this might work on some platforms, on others it doesn't:
Quote from http://msdn2.microsoft.com/en-us/library/s1sb61xd.aspx: "Assigning to a static local variable is not thread safe and is not recommended as a programming practice."
Although this isn't very concise wording (IIRC, the standard says that it should work for literals assigned to PODs) it seems clear that it won't work for non-PODs...
For assignation, sure, but I don't see any in my code...
As I said the wording isn't very concise. I think it's obvious that it will not work if state_dispatcher has a non-trivial ctor (which I assume it has).
However, if that's bother, I think then only something like Loki Singleton pattern will do. You then need a double lock for accessing it correctly, something like this:
static state_dispatcher const& get() { static state_dispatcher* pInstance; scoped_lock first_lock(m_mutex1) if (!pInstance) { scoped_lock second_lock(m_mutex2); if (!pInstance) { pInstance = new state_dispatcher; } } return *pInstance; }
From where did you get m_mutex1 & m_mutex2?? Where are they initialized?
-- Andreas Huber When replying by private email, please remove the words spam and trap from the address shown in the header.
Hello Andreas,
However, if that's bother, I think then only something like Loki Singleton pattern will do. You then need a double lock for accessing it correctly, something like this:
static state_dispatcher const& get() { static state_dispatcher* pInstance; scoped_lock first_lock(m_mutex1) if (!pInstance) { scoped_lock second_lock(m_mutex2); if (!pInstance) { pInstance = new state_dispatcher; } } return *pInstance; }
From where did you get m_mutex1 & m_mutex2?? Where are they initialized?
I guess there should have been only "scoped_lock second_lock(m_mutex2)" and no "scoped_lock first_lock(m_mutex1)". Still, m_mutex2 has to be initialized somewhere and I see no other way for it than to be global, in namespace scope. But once again, I'm not sure even this is actually safe since it's dynamic initialization which apparently may be done concurrently. -- Best regards, Andrey mailto:andysem@mail.ru
2007/2/5, Andrey Semashev <andysem@mail.ru>:
I guess there should have been only "scoped_lock second_lock(m_mutex2)" and no "scoped_lock first_lock(m_mutex1)". Still, m_mutex2 has to be initialized somewhere and I see no other way for it than to be global, in namespace scope. But once again, I'm not sure even this is actually safe since it's dynamic initialization which apparently may be done concurrently.
You're perfectly right on this issue: only the second_lock is necessary, and pInstance must also be volatile (I never write such code myself, I always used Loki implementation when I need a singleton). As for instanciation, the standard ensure that a local static variable will only be initialized once, before entrance to the block scope. How this is handled in multi-thread program? I guess it's depend on your threaded platform. As for where the mutexes is defined... Well, that's also depends on the class policy (and what's the mutex implementation permits). Unless you can ensure that the private variable will not be necessary anytime before it's initialization (for example, by a static object inside another module), I'm still thinking that the local static variable is safer than a class static variable. Fabien
AMDG Fabien Niñoles <fabien.ninoles <at> gmail.com> writes:
You're perfectly right on this issue: only the second_lock is necessary, and pInstance must also be volatile (I never write such code myself, I always used Loki implementation when I need a singleton). As for instanciation, the standard ensure that a local static variable will only be initialized once, before entrance to the block scope. How this is handled in multi-thread program? I guess it's depend on your threaded platform. As for where the mutexes is defined... Well, that's also depends on the class policy (and what's the mutex implementation permits).
The zero-initialization (8.5) of all local objects with static storage duration (3.7.1) is performed before any other initialization takes place. (6.7/4) A mutex is overkill. In this case a volatile class static with a trivial constructor and a volatile boolean flag is enough. In Christ, Steven Watanabe
Hello Fabien, Tuesday, February 6, 2007, 12:31:47 AM, you wrote:
2007/2/5, Andrey Semashev <andysem@mail.ru>:
I guess there should have been only "scoped_lock second_lock(m_mutex2)" and no "scoped_lock first_lock(m_mutex1)". Still, m_mutex2 has to be initialized somewhere and I see no other way for it than to be global, in namespace scope. But once again, I'm not sure even this is actually safe since it's dynamic initialization which apparently may be done concurrently.
You're perfectly right on this issue: only the second_lock is necessary, and pInstance must also be volatile (I never write such code myself, I always used Loki implementation when I need a singleton). As for instanciation, the standard ensure that a local static variable will only be initialized once, before entrance to the block scope. How this is handled in multi-thread program? I guess it's depend on your threaded platform. As for where the mutexes is defined... Well, that's also depends on the class policy (and what's the mutex implementation permits).
The problem is that the current Standard is not aware of threading at all. And the compiler follows it, making no effort to synchronize static variables initialization. So indeed, the local static variable is initialized once in a single-thread application. In MT you're on your own. You get the desired behavior only if you manage to protect the static variable initialization from being done in many threads simultaneously.
Unless you can ensure that the private variable will not be necessary anytime before it's initialization (for example, by a static object inside another module), I'm still thinking that the local static variable is safer than a class static variable.
I'm not quite sure for my case. -- Best regards, Andrey mailto:andysem@mail.ru
Hello Andreas, Monday, February 5, 2007, 11:17:32 PM, you wrote:
Fabien Niñoles wrote:
Probably you should put it has a static local variable inside a protected function, like this:
//! The method returns a reference to the only dispatcher instance
static state_dispatcher const& get() { static state_dispatcher instance; return instance; }
In a multithreaded app this might work on some platforms, on others it doesn't:
Quote from http://msdn2.microsoft.com/en-us/library/s1sb61xd.aspx: "Assigning to a static local variable is not thread safe and is not recommended as a programming practice."
Although this isn't very concise wording (IIRC, the standard says that it should work for literals assigned to PODs) it seems clear that it won't work for non-PODs...
I guess the only way to safely initialize a static local variable is through some call_once concept implementation. The problem in my case is that I'd like the library to be as light as possible and as fast as possible, so I can't make use of Boost.Thread for now. -- Best regards, Andrey mailto:andysem@mail.ru
AMDG Andrey Semashev <andysem <at> mail.ru> writes:
I guess the only way to safely initialize a static local variable is through some call_once concept implementation. The problem in my case is that I'd like the library to be as light as possible and as fast as possible, so I can't make use of Boost.Thread for now.
I believe that volatile works just as well here as it does for m_StatesInfo. template<class Dispatcher> struct static_event_dispatcher { static volatile bool initialized; //Dispatcher must have a trivial default constructor static Dispatcher dispatcher; static Dispatcher& value() { if(!initialized) { const_cast<volatile Dispatcher&>(dispatcher).init(); initialized = true; } return(dispatcher); } }; template<class Dispatcher> static volatile bool static_event_dispatcher<Dispatcher>::initialized = 0; template<class Dispatcher> static Dispatcher static_event_dispatcher<Dispatcher>::dispatcher; In Christ, Steven Watanabe
Steven Watanabe wrote:
AMDG
Andrey Semashev <andysem <at> mail.ru> writes:
I guess the only way to safely initialize a static local variable is through some call_once concept implementation. The problem in my case is that I'd like the library to be as light as possible and as fast as possible, so I can't make use of Boost.Thread for now.
I believe that volatile works just as well here as it does for m_StatesInfo.
template<class Dispatcher> struct static_event_dispatcher { static volatile bool initialized; //Dispatcher must have a trivial default constructor static Dispatcher dispatcher; static Dispatcher& value() { if(!initialized) { const_cast<volatile Dispatcher&>(dispatcher).init(); initialized = true; } return(dispatcher); } };
template<class Dispatcher> static volatile bool static_event_dispatcher<Dispatcher>::initialized = 0; template<class Dispatcher> static Dispatcher static_event_dispatcher<Dispatcher>::dispatcher;
As Andrei Alexandrescu explains in the following article: http://www.ddj.com/dept/cpp/184403766 a volatile-declared variable of primitive type only hinders the compiler to cache its value in a processor register. It doesn't prevent any race conditions in an MT application. In your code one thread can be in the middle of executing init() when another thread evaluates the condition in the if statement. Clearly, this will lead to two threads calling init(), possibly even at the same time. Even if you rewrite the code as follows // above same as before if(!initialized) { initialized = true; const_cast<volatile Dispatcher&>(dispatcher).init(); } // below same as before there's still a race condition. Two threads could evaluate initialized at the same time... All this even ignores weaker memory models of SMP machines where changes to initialized in one processor are not even visible to threads running on another processor unless you use a memory barrier. -- Andreas Huber When replying by private email, please remove the words spam and trap from the address shown in the header.
AMDG Andreas Huber <ahd6974-spamgroupstrap <at> yahoo.com> writes:
As Andrei Alexandrescu explains in the following article:
http://www.ddj.com/dept/cpp/184403766
a volatile-declared variable of primitive type only hinders the compiler to cache its value in a processor register. It doesn't prevent any race conditions in an MT application. In your code one thread can be in the middle of executing init() when another thread evaluates the condition in the if statement. Clearly, this will lead to two threads calling init(), possibly even at the same time.
The volatile is necessary to prevent the compiler from reordering the statements. Because init just does assignment of PODs it is ok if it is executed more than once. It is only a problem if executing x = y; in two threads at the same time does not work.
Even if you rewrite the code as follows
// above same as before if(!initialized) { initialized = true; const_cast<volatile Dispatcher&>(dispatcher).init(); } // below same as before
there's still a race condition. Two threads could evaluate initialized at the same time... All this even ignores weaker memory models of SMP machines where changes to initialized in one processor are not even visible to threads running on another processor unless you use a memory barrier.
This definitely bad. One thread could set the condition and another could then use dispatcher before it is initialized. In Christ, Steven Watanabe
Hello Steven, Tuesday, February 6, 2007, 12:10:17 AM, you wrote:
AMDG
Andrey Semashev <andysem <at> mail.ru> writes:
I guess the only way to safely initialize a static local variable is through some call_once concept implementation. The problem in my case is that I'd like the library to be as light as possible and as fast as possible, so I can't make use of Boost.Thread for now.
I believe that volatile works just as well here as it does for m_StatesInfo.
template<class Dispatcher> struct static_event_dispatcher { static volatile bool initialized; //Dispatcher must have a trivial default constructor static Dispatcher dispatcher; static Dispatcher& value() { if(!initialized) { const_cast<volatile Dispatcher&>(dispatcher).init(); initialized = true; } return(dispatcher); } };
template<class Dispatcher> static volatile bool static_event_dispatcher<Dispatcher>::initialized = 0; template<class Dispatcher> static Dispatcher static_event_dispatcher<Dispatcher>::dispatcher;
Well, the dispatcher is quite safe even if it's initialized more than once and even concurrently. Being a namespace-scope object is quite sufficient for it. But state names, which are std::strings, are quite fragile in this way. And unfortunately volatile flags won't help here. -- Best regards, Andrey mailto:andysem@mail.ru
AMDG Andrey Semashev <andysem <at> mail.ru> writes:
Well, the dispatcher is quite safe even if it's initialized more than once and even concurrently. Being a namespace-scope object is quite sufficient for it. But state names, which are std::strings, are quite fragile in this way. And unfortunately volatile flags won't help here.
You can use const char* or aligned_storage In Christ, Steven Watanabe
Hello Steven, Tuesday, February 6, 2007, 1:07:01 AM, you wrote:
AMDG
Andrey Semashev <andysem <at> mail.ru> writes:
Well, the dispatcher is quite safe even if it's initialized more than once and even concurrently. Being a namespace-scope object is quite sufficient for it. But state names, which are std::strings, are quite fragile in this way. And unfortunately volatile flags won't help here.
You can use const char* or aligned_storage
The problem is I don't know how to synchronize its initialization safely. Mutex construction should be synchronized too, that's the whole problem. I'll have think of it some more. -- Best regards, Andrey mailto:andysem@mail.ru
AMDG Andrey Semashev <andysem <at> mail.ru> writes:
The problem is I don't know how to synchronize its initialization safely. Mutex construction should be synchronized too, that's the whole problem.
The overhead of call_once is probably acceptable for constructing a string. In Christ, Steven Watanabe
Andrey Semashev wrote:
Hello Andreas,
Monday, February 5, 2007, 11:17:32 PM, you wrote:
Fabien Niñoles wrote:
Probably you should put it has a static local variable inside a protected function, like this:
//! The method returns a reference to the only dispatcher instance
static state_dispatcher const& get() { static state_dispatcher instance; return instance; }
In a multithreaded app this might work on some platforms, on others it doesn't:
Quote from http://msdn2.microsoft.com/en-us/library/s1sb61xd.aspx: "Assigning to a static local variable is not thread safe and is not recommended as a programming practice."
Although this isn't very concise wording (IIRC, the standard says that it should work for literals assigned to PODs) it seems clear that it won't work for non-PODs...
I guess the only way to safely initialize a static local variable is through some call_once concept implementation.
I guess so if you want to make it easy for the users. Another option would be to require them to call a static initialize function from main() before they start any additional threads. -- Andreas Huber When replying by private email, please remove the words spam and trap from the address shown in the header.
AMDG Andreas Huber <ahd6974-spamgroupstrap <at> yahoo.com> writes:
I guess so if you want to make it easy for the users. Another option would be to require them to call a static initialize function from main() before they start any additional threads.
That is not a good idea in this case. 1) Each instantiation of the template would need it's own initialize function. 2) It is not safe to use a state_machine during the dynamic initialization phase. In Christ, Steven Watanabe
Steven Watanabe wrote:
AMDG
Andreas Huber <ahd6974-spamgroupstrap <at> yahoo.com> writes:
I guess so if you want to make it easy for the users. Another option would be to require them to call a static initialize function from main() before they start any additional threads.
That is not a good idea in this case. 1) Each instantiation of the template would need it's own initialize function.
Right. 2) It is not safe to use a state_machine during the dynamic
initialization phase.
Correct, but at least some users might be willing to pay that price in return for a really lightweight and fast lib. -- Andreas Huber When replying by private email, please remove the words spam and trap from the address shown in the header.
Hello Steven, Tuesday, February 6, 2007, 12:25:35 AM, you wrote:
AMDG
Andreas Huber <ahd6974-spamgroupstrap <at> yahoo.com> writes:
I guess so if you want to make it easy for the users. Another option would be to require them to call a static initialize function from main() before they start any additional threads.
That is not a good idea in this case. 1) Each instantiation of the template would need it's own initialize function.
Not necessarily. The Standard requires _any_ function to be called from the TU to execute dynamic initialization.
2) It is not safe to use a state_machine during the dynamic initialization phase.
I was thinking about some functions that are executed automatically on module load (see __attribute__((constructor)) in GCC, for example). That might free users from calling such function. -- Best regards, Andrey mailto:andysem@mail.ru
Quote from http://msdn2.microsoft.com/en-us/library/s1sb61xd.aspx: "Assigning to a static local variable is not thread safe and is not recommended as a programming practice."
Although this isn't very concise wording (IIRC, the standard says that it should work for literals assigned to PODs) it seems clear that it won't work for non-PODs...
The example below the warning clearly show up what they mean, and it's really a concurrency situation where two threads access the same object. So, yes, this is a bad practice, and it is clearly not thread-safe, even for POD: // static3.cpp // compile with: /EHsc #include <iostream> using namespace std; struct C { void Test(int value) { static int var = 0; if (var == value) cout << "var == value" << endl; else cout << "var != value" << endl; var = value; } }; int main() { C c1; C c2; c1.Test(100); c2.Test(100); } Output var != value var == value
Fabien Niñoles wrote:
Quote from http://msdn2.microsoft.com/en-us/library/s1sb61xd.aspx: "Assigning to a static local variable is not thread safe and is not recommended as a programming practice."
Although this isn't very concise wording (IIRC, the standard says that it should work for literals assigned to PODs) it seems clear that it won't work for non-PODs...
The example below the warning clearly show up what they mean, and it's really a concurrency situation where two threads access the same object.
I don't see how the quoted example has any bearing on MT issues. It is a single threaded program.
So, yes, this is a bad practice, and it is clearly not thread-safe, even for POD:
I should have been clearer. The standard sais that *initializing* a POD is not a problem because that is done before any threads have a chance to race against each other (typically when the binary is loaded into memory). In the example, the second assignment to the POD is indeed a problem because it must happen at runtime. What I meant with my original statement is the following: class NonPod { public: NonPod() { /* ... */ } }; class Whatever { public: static int & GetPodInstance() { static int podInstance = 42; return podInstance; } static NonPod & GetNonPodInstance() { static NonPod nonPodInstance; return nonPodInstance; } }; When multiple threads are present, there's no race condition when more than one of them call GetPodInstance(). Clearly there is one when they call GetNonPodInstance() because NonPod::NonPod() will non run before the first call... -- Andreas Huber When replying by private email, please remove the words spam and trap from the address shown in the header.
AMDG Andreas Huber <ahd6974-spamgroupstrap <at> yahoo.com> writes:
I should have been clearer. The standard sais that *initializing* a POD is not a problem because that is done before any threads have a chance to race against each other (typically when the binary is loaded into memory). <snip>
static int & GetPodInstance() { static int podInstance = 42; return podInstance; } <snip> When multiple threads are present, there's no race condition when more than one of them call GetPodInstance(). Clearly there is one when they call GetNonPodInstance() because NonPod::NonPod() will non run before the first call...
GetPodInstance() is probably safe, but the standard does not require it. "A local object of POD type (3.9) with static storage duration initialized with constant-expressions is initialized before its block is first entered." (6.7/4) In Christ, Steven Watanabe
Steven Watanabe wrote:
When multiple threads are present, there's no race condition when more than one of them call GetPodInstance(). Clearly there is one when they call GetNonPodInstance() because NonPod::NonPod() will non run before the first call...
GetPodInstance() is probably safe, but the standard does not require it. "A local object of POD type (3.9) with static storage duration initialized with constant-expressions is initialized before its block is first entered." (6.7/4)
Interesting, from the wording above I would have concluded that the standard *does* require it. -- Andreas Huber When replying by private email, please remove the words spam and trap from the address shown in the header.
AMDG Andreas Huber <ahd6974-spamgroupstrap <at> yahoo.com> writes:
Steven Watanabe wrote:
GetPodInstance() is probably safe, but the standard does not require it. "A local object of POD type (3.9) with static storage duration initialized with constant-expressions is initialized before its block is first entered." (6.7/4)
Interesting, from the wording above I would have concluded that the standard *does* require it.
To be safe the compiler would have to initialize the object statically. i.e. before any code whatsoever is executed. The wording does not require that. In Christ, Steven Watanabe
Steven Watanabe wrote:
AMDG
Andreas Huber <ahd6974-spamgroupstrap <at> yahoo.com> writes:
Steven Watanabe wrote:
GetPodInstance() is probably safe, but the standard does not require it. "A local object of POD type (3.9) with static storage duration initialized with constant-expressions is initialized before its block is first entered." (6.7/4)
Interesting, from the wording above I would have concluded that the standard *does* require it.
To be safe the compiler would have to initialize the object statically. i.e. before any code whatsoever is executed. The wording does not require that.
I guess you are right regarding the wording but in practice such initialization takes place much earlier than the standard requires. Otherwise such a widespread library as pthreads would have a major problem: http://www.codesourcery.com/archives/c++-pthreads/msg00582.html -- Andreas Huber When replying by private email, please remove the words spam and trap from the address shown in the header.
Hello Steven, Wednesday, January 31, 2007, 10:02:24 PM, you wrote:
AMDG
First of all I think that the elapsed time is more useful than the number of iterations per second. I know that one is just the inverse of the other but the elapsed time has a linear correlation to the amount of work being done.
Second, ~half the time taken for the benchmark is the overhead of the test itself rather than the actual state machine. Try this version.
[snip]
Instead of using MPL vector/list you can create your own MPL sequence.
[snip] An updated version uploaded. -- Best regards, Andrey mailto:andysem@mail.ru
participants (4)
-
Andreas Huber -
Andrey Semashev -
Fabien Niñoles -
Steven Watanabe