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

[Tom Henderson]

Mathieu,
a few questions below:

Mathieu Lacage wrote:
> On Thu, 2008-10-16 at 17:30 -0700, craigdo at ee.washington.edu 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.

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?

> 
>> There is another module that lives down at the bottom of the
> hierarchy 
>> called the WallClockSynchronizer.  This establishes the actual relationship
>> with some real-time clock.  I expect that this may actually eventually be
>> replaced by something that looks at a time-stamp counter instead of making
>> system calls for the time-of-day clock.  In that case, you might have a
>> TscSynchronizer and no WallClock anything at all.  It seems to me that
> 
> Having a Tsc-based simulation backend does not necessarily exclude the
> possibility of implementing meaningfully the WallclockSimulator API. It
> just requires a careful measurement of the Tsc clock against the
> wall-clock time.
> 
>> putting the real-time scheduling methods into a class called
>> WallclockSimulator makes the wrong connection and stands out as, well, oddly
>> different than the rest of the conceptual model.  I decided it would be
>> better to leave the RealtimeSimulator name.
>>
>> Regarding the RealtimeSimulator class, you changed the method names from
>> ScheduleRealtime(), etc., to just Schedule().  I thought this would be a
>> mistake because there are already Simulator::Schedule methods.  Now there
>> are parallel RealtimeSimulator::Schedule methods (or Wallclock or whatever).
>> It seems to me that it is very easy to think that if you start using the
>> real-time simulator, you should just begin using the corresponding
>> RealtimeSimulator::Schedule() methods in your models.  This would be
>> disastrous!  They are absolutely not the same methods and they will
>> absolutely not do the same thing and if you follow the naming lead, your
>> code will begin to fail in very hard to debug ways.  Not good.
> 
> I removed the prefix because the prefix is already present in the class
> name: there is no confusion possible unless you move the methods back in
> the Simulator class which I proposed earlier but which you did not seem
> too excited about.

In earlier mail, you recommended:
 > 2) defines:
 > class WallclockSimulator
 > {
 > static void ScheduleXXX (Time delay, ...);
 > static void ScheduleXXXNow (...);
 > static Time XXXNow (void);
 > };
 >
 > 3) WallclockSimulator _API_ is documented to be thread-safe
 >
 > 4) implement Simulator and WallclockSimulator in the same file,
 > simulator.cc
 >
 > 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 
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?

Tom