mathieu.lacage at sophia.inria.fr
Fri Jul 11 08:24:27 PDT 2008
On Thu, 2008-07-10 at 20:43 -0700, craigdo at ee.washington.edu wrote:
> I admit it may seem ... strange ... to people who aren't intimately familiar
> with the concept of a critical section and this particular idiom, but then
> again I think it may help folks who don't know and don't want to learn too
> much about multithreading from screwing things up too badly, too easily.
> I'm pretty much always in favor of letting the compiler help me out. This
> idiom is to critical sections what smart pointers are to memory management.
> Perhaps some comments would help. Ultimately, I think my version of
> CriticalSection is much less error-prone than the manual locks. YMMV.
There is clearly a tension between being explicit and doing things
magically which are implicit. My take is that we already have too many
magic implicit things in our codebase and I would be much happier if we
could avoid adding more.
> > which will break if an exception is raised in the inner block, but,
> > well... I would be willing to live with this.
> I'll appoint you to be the person who fixes all multithreading problems and
> answers the phone at 02h00 to figure out why ns-3 sometimes hangs after 43
> hours of running a simulation. ;-)
I could live with that: I don't see it as a big potential risk and I am
willing to put my personal phone number here on the ML to back this
However, you seem to be feeling pretty strongly about this issue so, I
should point out that splitting your code as a separate simulation
backend in a separate file would go a long way towards convincing me to
leave this as-is because I could safely ignore it most of the time and
direct bug reports to you.
> [ ... ]
> > 2) rather than add more features to the
> > src/simulator/simulator.cc file,
> > I would have expected that we could make SimulatorPrivate public with
> > pure virtual methods (rename it maybe to SimulatorBackend) and provide
> > two implementations of that class: one which would be the same as the
> > current implementation and the other one which would be your
> > emu-enabled
> > implementation.
> I did think a lot about this, but believe that the overriding reality is
> that I really don't want to see two versions of one of the most basic piece
> of code in the system without putting up a fight. If we make two copies,
> they will diverge -- guaranteed -- and this will cause problems --
> guaranteed. This divergence will result in other phone calls at 02h00. You
> may have figured out that I don't like these phone calls very much.
I agree that your concern above is a valid concern but I am much more
concerned about making it as easy as possible to merge your code and
minimize the risk of merging. In that context, it is much less risky to
not touch our solid and well tested piece of code and add yours as an
option than to try to make the two work together.
In that specific case, there is another strong reason to do this: the
parallel work will require replacing all of that code with yet another
version of the mainloop so, we will have to go down that road at some
point and I see little point in pushing that further away.
> It turns out that the actual differences between the real-time and
> non-real-time versions are really quite small and I did localize them in
> replaceable implementations. The differences between the single-threaded
> version and multithreaded versions involve just the addition of the critical
> sections, which I am planning on making (transparently) replaceable with
> what are essentially no-ops as well.
> It seems a shame to me to just jump right in and fork the simulator core
> without at least trying to come up with a solution that integrates these two
> "relatively small" changes. If I don't succeed (for example, if I get stuck
> with a big performance impact I can't figure out how to address), then your
> suggestion is definitely the way to go and I'm not against it; I just don't
> think it's a first-choice thing.
I think that a better use of our energy would be to make the emulation
version focused on doing its job well when doing emulation rather than
trying to make its overhead small enough to use the emulation version
even when we don't do emulation.
Now, if, some day, your code becomes sufficiently 'efficient' and stable
to replace the default version and if we feel that we don't want to
maintain both versions, we can clearly get rid of the 'old' version but
that is a step I would like to avoid taking now.
More information about the Ns-developers