[Ns-developers] log time -> log context

[Gustavo Carneiro]

On 23/05/2008, Tom Henderson <tomh at tomh.org> wrote:
>
> Mathieu Lacage wrote:
>
>> On Wed, 2008-05-21 at 19:35 +0100, Gustavo Carneiro wrote:
>>
>>  I don't know what else to explain:
>>>
>>>  /**
>>>   * Set the current node that is being simulated.
>>>   * \warning this method has no effect on optimized NS-3 builds, as
>>> it is used only for logging.
>>>   * \param node   id of the current node, or -1 if none or not known
>>>   */
>>>  static void SetCurrentNode (int32_t node);
>>>
>>>
>>> It's dead simple.  What might be missing perhaps is some guideline
>>> explaining when the user is expected to call it.  Is that what you
>>> mean?
>>>
>>
>> What is "the current node" ? What is its relationship with the "nodeid
>> context" embedded in each event ? How does each event get a "nodeid
>> context" ? Where does it get it from ?
>>
>> There are many types of "current node id". There is the one which is
>> used to associate a "current node id" to the event scheduled with
>> Simulator::Schedule. There is the one which is coming from the "current
>> node id" of the event we are currently executing. There is the "current
>> node id" used for logging. Explaining clearly the relationship between
>> all these different types of "current node id" makes my brain hurt.
>>
>>         Using raw sockets outside of the context of an Application
>>>        Start
>>>        function makes little sense and is clearly not the way the
>>>        system was
>>>        designed to be used: yes, it can be made to work, yes, we have
>>>        examples
>>>        which do this, yes you like being able to do it but, as
>>>        discussed a
>>>        while ago in a bug report, _this is not the way the system was
>>>        designed
>>>        to be used_. If, when you do this, you do not get the best
>>>        logging
>>>        messages in the world without some extra work, such is life.
>>>
>>> About the extra work:
>>>
>>>  1. I am not convinced the extra work will be worth anything.  It
>>> doesn't improve readability in my opinion;
>>>
>>
>> It does not improve code readability. It makes the conceptual model of
>> the API much more clear and defines precisely and uniquely what the
>> "current node id" means by uniquely associating a single "current node
>> id" to each event.
>> What you are trying to suggest also goes against the design of the
>> current API which is to move _all_ per-event arguments to the Schedule
>> calls: we could have done what you suggested for all other arguments:
>>
>> Simulator::SetNextDelay (Time delay);
>> Simulator::SetNextArguments (...);
>>
>> Whether or not the above is better than what we have now is not
>> something I will argue about. What I will argue about is consistency of
>> the design: we are not using this style of API in this header _now_ so,
>> we should not use this style for new API in this header.
>>
>>   2. Adding extra Schedule calls will increasing the number of events
>>> and decrease NS-3 performance.  I want the logging improvements to be
>>> as little intrusive as possible, as all logging should be.
>>>
>>
>> You are worried about minimizing the number of changes to the codebase.
>> I am worried about making the API understandable and consistent.
>>
>> Mathieu
>>
>>
>>
> What about asking each file to define an optional context that is
> meaningful to it, and leave it out where it doesn't make sense?
>
> e.g.
>
> in ipv4-l3-protocol.cc
>
> NS_LOG_COMPONENT_DEFINE ("Ipv4L3Protocol");
> #define APPEND_CONTEXT                                          \
>  if (m_node)                                                   \
>    {                                                           \
>      std::clog << m_node->GetId () << " ";                     \
>    }                                                           \
>
> then in log.h
>
> #define NS_LOG_FUNCTION_NOARGS()                                \
>  do                                                            \
>    {                                                           \
>      if (g_log.IsEnabled (ns3::LOG_FUNCTION))                  \
>        {                                                       \
>          APPEND_TIME_PREFIX;                                   \
>          APPEND_CONTEXT;                                       \
>          std::clog << g_log.Name () << ":"                     \
>                    << __FUNCTION__ << "()" << std::endl;       \
>        }                                                       \
>    }                                                           \
>  while (false)


That is not a bad idea but requires modifications in practically all C++
source files.  My approach requires much less modifications, namely:

  1. In places where there is a transition between node, in particular in
channels;
  2. In places where there is a transition from user scripting context to an
object associated with a specific node (e.g. Sockets, Applications).

It turns out the effort required by developers to support this node context
is minimized this way.  Unless the developer is going to develop a new
NetDevice or a new Application, most likely the developer does not even need
to be aware of this node context and everything "just works".

-- 
Gustavo J. A. M. Carneiro
INESC Porto, Telecommunications and Multimedia Unit
"The universe is always one step beyond logic." -- Frank Herbert