Re: [boost] [FSM] Review library version update

From: Steven Watanabe (steven_at_[hidden])
Date: 2007-02-03 14:05:52

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