[boost] Re: review of FSM library starts

From: Andreas Huber (ahd6974-spamgroupstrap_at_[hidden])
Date: 2005-03-11 18:16:54

Hi Alexander

Thanks for your review!

> Most things are quite understandable. However, code snipsets are quite
> complex. On my first reading, I stopped trying to understand them
> after a second of third sample.
> May be it's only me who reads complex stuff in a such way but the fact
> that simple_state class accepts up to five parameters gives a hint to
> a user that there shall be not_so_simple_state class that accepts
> 6+ or is more complex in other ways.

Actually I never liked the name of the simple_state class template but I
can't really think of a better one. The rationale is: You can do
everything with state, "simple_state" should suggest that you can do
less with it. It is difficult to pack the real difference in a short
descriptive name. Suggestions welcome.

> This appears at the beginning and I suspect some will stop reading
> the tutorial right at that point.
> I'd suggest using some smart intendation at least.

There's going to be an overhaul of the tutorial, including the
indentation.

>>     * What is your evaluation of the design?
>>       What features are supported better by
>>       alternative FSMs? What could be added
>>       (or removed) from the library?
>
> I don't like the design. Primary goal of the library is to be
> close to UML and the secondary goal is performance. UML
> conformance is pushed so strongly that any attempt to improve
> performance makes design worse.

It would be interesting to hear how you came to that conclusion. The UML
conformance has *nothing* to do with the current state of affairs
regarding performance. I think it is possible to write a UML conformant
FSM lib that has the often quoted "ruthless" efficiency. The main reason
for the perceived "bad" performance are the scalability requirements.
And believe me: There are applications that absolutely need that
scalability.

> For example, deferral<XXX>
> requires that event is allocated using new and pointed by
> intrusive_ptr. Ordinary events can be allocated differently.
> If performance was out of library scope, the library could copy
> _all_ events into heap-allocated buffer. From user point of view,
> all event kinds looked the same, then.

I don't understand. The requirement exists exactly because anything else
would be less efficient (space, time, executable size). Users who don't
care can heap-allocate all events anyway.

> I believe that performance is not so important because even optimized,
> the library is too slow for FSM.

Too slow for Finite State Machines? I can't make any sense of this.

> According to docs, process_event
> function has 10 steps and there is a search in a couple of steps.
> This means loops and branching.

Don't take this description too literally. Much of this searching could
theoretically be eliminated. Some branches can be (and are) done at
compile time.

> I always thought that process_event
> in typical FSM is single jump followed by indirect function call.

Yes, that's the common way to implement simple FSMs. Problem is: You
don't get very far that way when your FSMs become bigger.

> There are other things I don't like. One example is that transit call
> in react is similar to "delete this;"

Please suggest an alternative.

> The  "Unstable state" and "Unstable state machine" sections are too
> among things I don't like. They exist because transition is a complex
> process which involves multiple destructions and constructions.

No. Any state machine can become unstable as soon as you have both of
the following:
1. Hierarchical states
2. Failing actions (doesn't matter what: entry, exit, transition)

This has *nothing* to do with the interface or the implementation of the
proposed FSM library.

If you were to use the library and you don't want your FSM to become
unstable, then either:
- Don't let actions fail
- Don't use hierarchical states
- Always use null_exception_translator

You really do have a choice here!

> Not only it affects performance

Yes, it does affect performance *slightly*, but what good is performance
when your code isn't correct?

> but it complicates recovering
> from exception.

The exact opposite is true. Please explain how you came to this
conclusion.

>>     * The library documentation contains
>>       few not yet solved issues (name,
>>       separating the library into two parts,
>>       exception handling). What is you opinion here?
>
> I'm stongly against name FSM

As I said: I'm not against changing the name

> because the library doesn't give enough
> guarantees common in FSM world.
> Because the library is not predicitable in real-time terms

I suggest you pinpoint the locations in the code that you think cause
non-predictability.

[snip]
>>     * Are there performance bottlenecks?
>>       Does the library design fit requirements
>>       of real-time systems? How is it useable
>>       for embedded systems?
>>       Is the documentation clear on performance
>>       tradeoffs?
>
> I don't think it's acceptable for real-time systems.

We seem to have different definitions of what real-time means. Real-time
does *not* mean fast. Real-time means that you can specify an upper
limit of how much time it will take to process an event. I don't think
anything in the proposed FSM library prevents you from doing that.

>>     * What is your evaluation of the potential
>>       usefulness of the library? Can you compare
>>       this FSM to other implementations?
>
> It's definitely useful as a library for representing
> UML statechart but it covers only subset of FSM.

You mean it only covers the subset of FSMs that don't need ruthless
efficiency? I could certainly agree with that.

[snip]
> I'm afraid not. Either it should be repositioned as Boost.Statecharts
> or redesigned for better real-time support.

See above.

Regards,

-- 
Andreas Huber
When replying by private email, please remove the words spam and trap
from the address shown in the header.