Re: Process Tracing proposal

Mark, All,

I had a quick look through your proposal. Generally speaking, I would
like to see something like this in the next 1666 revision.

Nevertheless, I have some comments on the current proposal.
Sorry for the quite lengthy mail. :-)

Contents:
  1. Performance impact
  2. Feedback from CCIWG
  3. Parameter types
  4. Registering/unregistering an sc_process_tracer
  5. Tracing granularity
  6. sc_event callbacks, immediate notification
  7. Process callbacks
  8. sc_event_list | sensitivity API
  9. sc_event_iterator nit-pick

1. Performance impact

   Since the tracing calls are on the critical path in the simulator
   (every event notify/trigger, process state change), there may be
   a performance degradation even when no observers are registered.
   Do you have benchmarks on this? Should we add a preprocessor
   symbol to explicitly activate this feature, like
   SC_INCLUDE_PROCESS_TRACING?

2. Feedback from CCIWG

   This feature is closely related to the OSCI CCI working group, since
   it helps for model introspection. Although the CCIWG currently works
   on configuration, it may be reasonable to get feedback from them as
   well.

3. Parameter types

   The callbacks in the proposed API currently get their parameters by
   pointer to non-const. I would prefer to pass events by const
   reference and processes via an sc_process_handle param:

   void event_triggered( sc_event const & );
   void method_started( sc_process_handle );

   Rationale:
   - The event callbacks should not mess with the event right? Calling
     ev->cancel() from event_triggered() may confuse the simulator.
   - sc_process_b is not part of IEEE 1666-2005. Processes should be
     accessed via the handle class (which can safely passed by value).

4. Registering/unregistering an sc_process_tracer

   add|remove_process_tracer in class sc_simcontext is an implementation
   artefact, right? I think, the registering/unregistering should be
   done in the constructor/destructor of the tracer base class
   automatically.

> * All tracers, which were added to the sc_simcontext,
> * and not removed before the end of simulation, will be deleted by
> * sc_simcontext.

   This should NOT be the case, as it obliterates ownership relations
   between the creator of the tracer and the simulation kernel.

5. Tracing granularity

   The proposed tracing is a centralised mechanism for events _and_
   processes. Two questions on this:

   - Would it be reasonable to split this in two interface classes
     sc_event_tracer, sc_process_tracer? (a particular tracer could
     still implement both interfaces in a single class).

   - If a model is only interested in tracing a particular subset of
     processes/events, the filtering has to be done inside the tracer.
     An interface to add a tracer to an sc_event/sc_process_handle
     explicitly could increase the flexibility here.

   For convenience, I would implement the callback as no-ops in the base
   classes and only leave the destructor pure virtual to enforce
   implementation:

   struct sc_process_tracer
   {
     virtual void event_notified( sc_event const & ) {}
     virtual ~sc_process_tracer() = 0;
   };
   inline sc_process_tracer::~sc_process_tracer(){}

   Otherwise, every user-defined tracer needs to implement all
   functions, even if only a subset of callbacks are needed.

6. sc_event callbacks, immediate notification

   I think, the notify_t needs another value IMMEDIATE, for immediate
   notification of events within the same delta. Here, it needs to be
   defined, when the callbacks are executed.

   Consider the following SC_METHOD(foo):
   {
     // do something
     ev.notify(); // immediate notification
     // do something else
   }

   Callbacks involved (all in the same delta cycle):
     method_started( foo );
     event_notified( ev );
     method_finished( foo );
     event_triggered( ev );

   What order do we expect between the event_* and the method_*
   hooks here?

7. Process callbacks

   For the process related callbacks, a more elaborate definition of
   the execution ordering between different callbacks and relative to
   execution of process code should be defined. e.g. 'thread_resumed()'
   -> 'after resuming a thread', but before or after actually running
   it? 'after a thread was reset' -> before or after restarting it?

   What is 'normal termination'? How should the tracer react to
   exceptions? I think, exceptions (not only sc_report) should be
   caught and re-thrown after invoking the termination callback, but
   at least it should be defined.

   Are all termination callbacks guaranteed to be executed at the end
   of the simulation? Are processes "killed" then?

8. sc_event_list | sensitivity API

  This is related to having a more flexible way to access event lists,
  as previously discussed in this group. I have the feeling, the
  proposed API for event lists could be sufficient, although explicit
  size() and maybe empty() functions for the list would be useful
  especially if only a forward-iterator interface is provided.

  Are sc_event_or_list, sc_event_and_list then meant to inherit from
  sc_event_list? Should an sc_event_list know about it being an
  "and" or "or" list?

  The API to query the static/dynamic sensitivity looks quite ad-hoc to
  me, though. I don't really like passing references to pointers
  around and prefer clear ownership and lifetime definitions.

  So I would replace
    sc_event_iterator static_sensitivity_begin_iterator() const;
    sc_event_iterator static_sensitivity_end_iterator() const;
  with a single
    sc_event_list static_sensitivity() const;

  and

  trigger_t get_dynamic_sensitivity(sc_event const *&event_p,
                                    sc_event_list const *&event_list_p,
                                    sc_time &notify_time) const;

  with something like

  struct sc_sensitivity {
    trigger_t kind; // maybe move into sc_event_list?
    sc_event_list events;
    sc_time notify_time;
  };

  sc_sensitivity dynamic_sensitivity() const;

  These functions need to be added to the process-handle class, as well.

9. sc_event_iterator nit-pick

  - operator* should return a reference, instead of a pointer:
    (*it)->foo() feels unnatural to me, since it is an event-list, not
    an "event-handle-list".

  - sc_event_iterator could derive from
      std::iterator< std::forward_iterator_tag, sc_event >
    for better integration with the C++ standard library
    (maybe std::iterator< std::bidirectional_iterator_tag, sc_event > ?)

Greetings from Oldenburg,
Philipp

On 26/04/10 05:29, Glasser, Mark wrote:
>
> As I mentioned at the recent P1666 meeting, Mentor has a proposal for a
> process tracing feature to be included in the next 1666 revision. The
> feature is essentially a set of callbacks which are invoked as processes
> change state. We would like to get feedback on the proposal from
> members of this discussion group.

-- 
Philipp A. Hartmann
Hardware/Software Design Methodology Group
OFFIS
R&D Division Transportation | FuE-Bereich Verkehr
Escherweg 2 · 26121 Oldenburg · Germany
Phone/Fax: +49-441-9722-420/282 · PGP: 0x9161A5C0 · http://www.offis.de/
-- 
This message has been scanned for viruses and
dangerous content by MailScanner, and is
believed to be clean.