[Ns-developers] merge of rts/wallclock simulator code

[Mathieu Lacage]

On Sat, 2008-10-18 at 13:52 -0700, Tom Henderson wrote:

> >> The basic architecture is pimpl.  There is an interface class called
> >> Simulator.  There are two implementations.  The implementation for the
> >> real-time simulator is called realtime-simulator-impl.  In order to switch
> >> to the realtime-simulator-impl you bind ns3::RealtimeSimulatorImpl to the
> >> SimulatorImplementationType global variable.  It's clear that there are two
> >> choices throughout the architecture, the default simulator implementation or
> >> the real-time simulator implementation.  There's not a wall-clock thing in
> >> there anywhere, so why should the extension class be called
> >> WallclockSimulator?  It seems off in left field to me.
> > 
> > Because, as I said in a previous email, I intend to implement
> > WallclockSimulator correctly in both the non-realtime and the realtime
> > case. Lots of comments below are related to this issue: It seems I had
> > not made that intent clear. 
> 
> Can you please elaborate on the above?  I went back in your previous 
> mail and didn't find this explanation.  What do you mean that you intend 
> to implement WallclockSimulator for the non-realtime case?  Do you mean 
> a thread-safe non-realtime simulator?  Non-realtime Wallclock seems 
> contradictory.

We have two APIs: simulator.h and wallclock-simulator.h. The former
(API) makes no commitment about realtime semantics and/or thread-safety.
The latter (API) does. We have two implementations of both APIs:
  1) the default implementation implements simulator.h without any
realtime semantics and _could_ implement wallclock-simulator.h with
realtime semantics (it's not very hard).
  2) the realtime implementation implements simulator.h with realtime
semantics _and_ wallclock-simulator.h with realtime semantics.

Craig proposed the new API based on my report of a bug about the
realtime implementation of Simulator::ScheduleNow.  Craig came up with a
new API exported from the simulator module only when the realtime
simulator is enabled. 

So, while I really don't care how the realtime simulator is implemented
provided the bugs I report are fixed, the issue, here, is that adding
new API is an entirely different business and that IMO requires some
things that are pretty reasonable:
  a) you have to provide a use-case for that API
  b) that API should try to be minimal for that use-case
  c) that API should work across all simulator implementations

My comments and simulator repo were aimed at addressing the above. 

Now, the question is how we can go forward:
  1) I care about the internal cleanups I did in the event schedulers,
so, I will push these

  2) I really don't care about wallclock-simulator.h because it is
neither minimal for our use-case nor implemented for all backends and it
seems Craig does not like it and I don't like it very much anyway.

  3) I mentioned a couple of times in irc discussions with craig my
dislike for the locking code in the EventImpl base class. I would like
to make sure that this code disappears eventually and that should be
trivial because there is no reason to make the realtime implementation
of the simulator.h API thread-safe since the non-realtime version is not
thread-safe.

  4) someone will have to figure out how to make the emu/tap devices
work and what kind of minimal extra API they need.

The hard problem is 4) and, I think that we should take a step back and
define again the use-case we are trying to deal with. Specifically, it
is clear to me that we have two requirements:

  i. we need to be able to insert an event into the main Simulator::Run
thread from another thread. This is a thread-safe Schedule-like method.
That method should not return an EventId to allow us to fix (3) above.

  ii. we need to make sure that the simulation time will increase when a
real-world event is triggered (a real-world packet is coming in).

The main problem is ii: it requires that when an external event is
scheduled, it is scheduled with a delay equal to the wallclock delay
since the last event was executed. This is, I believe, what craig
implemented in RealtimeSimulatorImpl::ScheduleRealNow.

The issue, though, is that implementing all this logic and functionality
in the realtime simulator code requires us to define new API which is
realtime-specific and, again, I would like to avoid this. I might be
wrong but I think that a way to do so would be something like the
following API:

// monitor callback is Invoked synchronously by Simulator::Run 
// before executing an event and changing the current simulation time
void Simulator::SetPreEventHook (Callback<void,EventId>);
void Simulator::ScheduleThreadSafe (Time delay, ...);

Using it as follows (not very clean pseudo-code):

EmuNetDevice::EmuNetDevice ()
{
  Simulator::SetPreEventHook (&PreEventHook);
}
void 
EmuNetDevice::PreEventHook (EventId eid)
{
  m_lastEventTimeLock.Lock ();
  m_lastEventTime = GetTimeOfDay ();
  m_lastEventTimeLock.Unlock ();
}

void
EmuNetDevice::ReadThread (void)
{
 while (true) {
  buffer = read ();
  m_lastEventTimeLock.Lock ();
  Time delta = GetTimeOfDay () - m_lastEventTime;
  Simulator::ScheduleThreadSafe (delta, &ForwardUp, buffer);
  m_lastEventTimeLock.Unlock ();
 } 
}

The above would need a sample implementation to validate that it really
works but it seems to me that it should work and define a
non-realtime-specific API. That API would also make it relatively easy
to implement a terminal-based (or gui-based) helper class which prints
on screen a status update about the current simulation time every
wallclock second or so.

> Can you provide some kind of overview of the big picture of where you 
> see the simulator (i.e. class Simulator) evolving, past this merge?

Here are features I have in mind:
  - add support for more event schedulers (raj said he would work on
this. I might rip out some ns2 code)
  - eventually implement a parallel simulator backend which implements
the gsoc project I sent here a couple of months ago

The former has no API impact. The latter has API and implementation
impacts which were described back then in the gsoc project description.

Other than that, I have no real plans. The cleanups I pushed in the
ns-3-simulator tree are mostly things raj had been complaining about
(the fact that event schedulers did not share code they should be
sharing and the fact they had to do some non-trivial memory
management ).

>  > 5) extend SimulatorImpl:
>  > class SimulatorImpl
>  > {
>  >  virtual void ScheduleXXX (Time delay, ...) = 0;
>  >  virtual void ScheduleXXXNow (...) = 0;
>  >  virtual Time XXXNow (void) const = 0;
> 
> But now you are suggesting to drop the postfix.  This seems to be
> going 

Yes: I do make mistakes sometimes.

> against the grain of your typical recommendation to use different
> names 
> for functions that have different semantics.  Why are you
> recommending 
> now that WallclockSimulator::Schedule instead of 
> WallclockSimulator::ScheduleXXX?  You mention the context being 
> sufficiently clear, but why not make it clearer?

I can't see how WallclockSimulator::ScheduleWallclock is clearer than
WallclockSimulator::Schedule. It is more verbose, yes. Clearer ? I don't
think so. Yes, this is a totally subjective statement, I know.

regards,
Mathieu