[Ns-developers] About real-world application integration

[Hajime Tazaki]

At Mon, 16 Mar 2009 11:11:09 +0100,
Mathieu Lacage wrote:

>Anyway, I think you can leave this in for now: the tagging code is going
>to undergo changes for the ipv4 rework so, this will probably be solved
>during in the ipv4 rework.

I see.

>> >4) I see a couple of patches to src/process-manager/* which could be
>> >merged in ns-3-simu right now, so, it would be nice if you could try to
>> >split them from the rest to try to minimize the number of differences
>> >between your tree and ns-3-simu. For example, the following looks like a
>> >good idea and I would be happy to take patches to add them to
>> >http://code.nsnam.org/mathieu/ns-3-simu
>> 
>> Okay, I'll try it later.
>> Do you mind if I'll send patches for this modification?
>> Or using bugzilla is better?
>
>you can:
>  - create a mercurial repo with only the patches you want to send me
>and I will pull from this repo, review, and push in mathieu/ns-3-simu
>
>  - or send me raw patches by email (mercurial has a patchbomb
>extension:
>http://www.selenic.com/mercurial/wiki/index.cgi/PatchbombExtension)

Okay, thanks.
I'll tell you when I can prepare it.

>> >and, I assume that this made a bit difference for you:
>> >
>> >-  std::vector<std::pair<int,Ptr<UnixFd> > > openFiles;
>> >+  __gnu_cxx::hash_map<int, Ptr<UnixFd> > openFiles;
>> >
>> >but, oh, look at src/internet-stack/sgi-hashmap.h: would you mind move
>> >it to src/core and use this one instead ?
>> 
>> I don't mind, I just wanna use hash_map, but I wasn't sure
>> how I can do it, so just trid to do same as
>> src/internet-cache/arp-cache.h.
>> 
>> BTW, using hash_map comes from oprofile output to improve
>> performance, but it makes not so big differences.
>
>If it makes no real difference, then, maybe we can drop this ? I really
>have never tried to profile the behavior of the process-manager code so,
>I have no idea how it behaves: I would tend to trust you if you did
>profile it and if you think it is worth being done :)

I agree.
Although this modification makes efficiency, I will drop
this.

>> >The following looks a bit gratuituous:
>> >
>> >diff -r 89718962b257 -r 6066282751f1
>> >src/process-manager/process-manager.cc
>> >--- a/src/process-manager/process-manager.cc    Fri Feb 27 23:57:07 2009
>> >+0900
>> >+++ b/src/process-manager/process-manager.cc    Wed Mar 11 15:03:07 2009
>> >+0900
>> >@@ -109,12 +109,12 @@
>> > {
>> >   NS_LOG_FUNCTION (this);
>> >   GarbageCollectDeadProcessesAndThreads ();
>> >-  for (std::vector<struct Process *>::iterator i = m_processes.begin
>> >(); i != m_processes.end (); ++i)
>> >+  for (std::vector<struct Process *>::iterator i = m_processes.begin
>> >(); i != m_processes.end ();)
>> >     {
>> >       struct Process *process = *i;
>> >       DeleteProcess (process);
>> >+      i = m_processes.erase (i);
>> >     }
>> >-  m_processes.clear ();
>> 
>> I can make a patch, too.
>
>I meant that I don't really understand what this patch is doing: it
>seems that it's really doing the same thing as before, only differently.

Ah, sorry.

When DeleteProcess is called, and threads is not empty,
DeleteProcess hook SIGKILL in the context of DoDispose(),
and delivered to simu_exit(). Then process is removed from
m_processes before returning DeleteProcess().

So if there is multiple processes in m_processes, it might
be problem. This patch try to fix this.

>> Is "indent -gnu {all files}" good enough?
>
>I honestly have no idea. I usually do all the coding style formatting by
>hand. The emacs gnu mode helps but it does not do everything.

Okay, thanks.

regards,
hajime