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.