[Ns-developers] merge of rts/wallclock simulator code
craigdo@ee.washington.edu
craigdo at ee.washington.edu
Thu Oct 16 17:30:55 PDT 2008
> code is online now thanks to raj's work:
>
> http://code.nsnam.org/mathieu/ns-3-simulator
>
I took a look at this repository and it's mostly what we talked about. I do
have some comments.
You wanted to change the name of the extension class to WallclockSimulator.
After some reflection, I decided that wasn't the best thing to do.
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.
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
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 decided it would be better to keep the name of the methods in the new
class different (e.g., RealtimeSimulator::ScheduleRealtime, to reinforce
that they are different methods, with different semantics to be used in
different places for different reasons. Perhaps it looked odd to you at
first glance, but it was there for IMO an important reason. Maybe if you
don't like the at-first-glance redundancy of the name, we could reinforce
that these are the multithread safe-methods and call it
MultithreadSimulator::ScheduleRealtime or something like that. I cringed
when I saw RealtimeSimulator::Schedule().
You also removed the GetRealtimeNow method. I think this is a useful method
and I'm not sure why you wanted to get rid of it. It is useful, for
example, in a scenario you wanted to support -- RunOneEvent. Since there is
no real-time synchronization if an external event loop is in charge, it
seems useful to be able to give that external loop a real-time base to work
with. It would be incredibly convenient to provide a real-time that is
nicely represented as Time units. If it's not there, the client gets to
convert from Unix native time structures back and forth to ns-3 times
through doubles and seconds or some such conversion path.
GetCurrentRealtime seems extremely convenient for this and useful for
timestamping events or quick performance measurements.
Regarding the event impl stuff, you did basically what I was in the process
of doing. Seems like it will work just fine, but I wanted to do some
serious testing before I tossed it out. I've changed the main-test-sync
program to reflect the new multithread, non-multithread distinction and have
let it beat on your code for a while and it seems solid.
I am curious about why you decided to change the relationship between the
RealtimeSimulator/WallclockSimulator and Simulator from the "friend"ly
relation I had implemented.
We have a central interface class Simulator with a replaceable
implementation. One of the implementations has some new methods that I
don't think are really meaningful in the other (like ScheduleRealtimeNow or
whatever you decided to call it). It made good sense to me to have a friend
class of Simulator that used all of the nifty templates and such in the
central Simulator class and provided access to the new methods by vectoring
them to the real-time implementation where they were meaningful. If you
weren't using the real-time implementation I asserted something to the
effect of "you can't do that unless you have the real-time simulator
implementation selected". This seemed to me to fit nicely with the
architecture and conceptual model, and provided a meaningful error message.
You pulled the common templated stuff out of Simulator and made Simulator
and RealtimeSimulator peers. You added real-time multithread-semantics
scheduling methods to the default implementation (where I really don't they
are meaningful) and then plumbed the real-time calls to both
implementations. The real-time implementation does what it should, but you
errored out if the new methods are called in the default implementation.
The end result is that the user just sees "not implemented".
This all seems a bit less straightforward to me. You obviously spent some
considerable amount of effort to make all of these changes, and I'm
interested in why you felt all of that effort was worthwhile.
-- Craig
More information about the Ns-developers
mailing list