[Ns-developers] [ns-3] binding Packet/Tag/TraceContext

[Mathieu Lacage]

On Tue, 2008-01-08 at 17:14 +0000, Gustavo Carneiro wrote:
> On 08/01/2008, Gustavo Carneiro <gjcarneiro at gmail.com> wrote:
>         On 08/01/2008, Mathieu Lacage <mathieu.lacage at sophia.inria.fr>
>         wrote:
>                 On Tue, 2008-01-08 at 12:00 +0000, Gustavo Carneiro
>                 wrote:
>                 
>                 >         2) non-virtual methods are going to be made
>                 virtual:
>                 >           Header::Serialize, etc.
>                 >
>                 > Do you mean to go back to the original Header class,
>                 which contained 
>                 > virtual methods, before you made it an empty class
>                 and switched to
>                 > using templates?  That would sound great to me.  Not
>                 only for Python
>                 
>                 yes.
>                 
>                 >  but also for pure C++ point of view...
>                 
>                 I personally hate the idea of doing this if only
>                 because it means that I
>                 get to rewrite all this code, once more which does not
>                 make me very
>                 happy. I want to know what the alternatives are first.
>         
>         The alternative is to define a PythonHeader class containing
>         the virtual methods, and use it only for python.  Then the C++
>         API remains unchanged.


It is not going to work because C++ stores a uid into the metadata
stream which identifies the C++ _type_ of the header pushed into the
packet. Here, since the C++ type/uid is always the same for all python
headers, the C++ code won't be able to tell the difference between two
python headers being added to a Packet. Hence, the need for the python
code to augment the C++ mapping table with a pair uid <-> instance
creation function. Basically, whenever python detects that there is a
new python header type being created, it should register in the C++
runtime a uid and a C++ function which returns a pointer to an instance
of that new python header type . i.e., something like this:

// C++ function provided to you by the ns-3 runtime
// The C++ code needs to be able to delete the Header pointer
// returned by this function registered below.
uint32_t RegisterDynamicHeader (Header *(*) (void));

Now, if you do all of this, what the C++ code needs is a way to know,
for each PythonHeader instance, its uid, even though it is the same C++
type. To get this, the C++ code needs a virtual GetUid method which
means that you need to change the C++ Header API to have a virtual
GetUid. Once you are there, it is just simpler to make all methods
virtual, hence the need for a complete API change...
>         
>                 >         3) Tags and TraceContextElement are going to
>                 require an
>                 >         explicit
>                 >         serialization/deserialization API. i.e.,
>                 something a bit like
>                 >         Header::Serialize/Deserialize
>                 >
>                 > For tags, I think they are already being serialized
>                 under the hood,
>                 > right?  Just a matter of exposing an _additional_
>                 API with it.
>                 
>                 They are serialized through memcpy which is not going
>                 to work if we use 
>                 c++ classes with virtual methods, hence, the need for
>                 an extra pair of
>                 explicit Serialize/Deserialize methods which write and
>                 read the tag
>                 data
>         
>         That sounds fine for the Python bindings, but awful for the C
>         ++ API.  If I have to implement Serialize/Deserialize I will
>         think very hard before implementing a new tag.  Please don't
>         do this.
> 
> Correction: Serialize/Deserialize is actually _needed_ for supporting
> Python tag objects.  But it is not _wanted_ for C++ ones.  The obvious
> solution would be to optionally support Serialize/Deserialize,
> fallback to memcpy when not available. 

Unfortunately, given the way this is implemented, there is no way to
parameterize whether you should memcpy or not. So, if you want a
Serialize/Deserialize, you need to implement it for everyone. Anyway, I
don't think that it is a big deal that you have to implement
Serialize/Deserialize for C++ tags: these methods should be pretty
trivial 3-liners so, it is not something I am worried about: yes, it is
work but it is a rare event that you can get something for free.

> 
> Although I am concerned that serialization of a python object is going
> to be inneficient; python tag objects will likely serialize (I am
> thinking of using pickle) to a rather large byte array.  Since tags
> have a small fixed maximum size, either the size will have to be
> increased or else we won't support python tag objects. 

You could store in the C++ tag a pointer to your python data. The memory
management of this pointer can be trivially done if you implement
carefully the copy constructor and destructor of the C++ PythonTag class
because both of these operators will be invoked when the PythonTag is
copied into the Tags data structure and when it is deleted from the
structure. Remember, you are the one who asked for this feature a long
time ago :)
> 
> That's why it would be much more interesting if tags were instead
> reference counted objects that would be passed around and which would
> have a Clone () method for when packets are copied.


I do not think you need this because your tag's copy constructor and
destructor are invoked when you call Packet::AddTag and  when the last
Packet which references this tag is destroyed.
> 

[snip]
>         
>                 def MyPythonFunction(context, ...):
>                   index = new NodeListIndex ()
>                   context.GetElement (index)
>         
>         I guess that could work. 
>         
>                 All you have to do is to not wrap the C++
>                 NodeListIndex directly.
>                 Instead, you can wrap this:
>                 
>                 class PythonTraceContextElement
>                 {
>                 public:
>                   virtual bool ReadFrom (const TraceContext &context)
>                 = 0;
>                 };
>                 
>                 class PythonTraceContextElementNodeListIndex : public
>                 PythonTraceContextElement
>                 {
>                 public:
>                   uint32_t GetNodeId (void) const {
>                     return m_elem.Get ();
>                   }
>                 private:
>                   bool ReadFrom (const TraceContext &context) { 
>                     bool found = context.GetElement (m_elem);
>                     return found;
>                   }
>                   NodeListIndex m_elem;
>                 };
>                 
>                 class PythonTraceContext
>                 {
>                 public:
>                   bool GetElement (PythonTraceContextElement *elem) {
>                     return elem->ReadFrom (m_context); 
>                   }
>                 private:
>                   TraceContext &m_context;
>                 };
>                 
>                 And, then, when the user creates a python object of
>                 type NodeListIndex,
>                 you create a C++ object of type
>                 PythonTraceContextElementNodeListIndex
>                 and wrap _that_ object.
>         
>         I don't think it needs something so complicated.   All
>         PyBindGen needs is to map each instance of the GetElement<T>
>         method into the same method name.  Then C++ overloading will
>         kick in and ensure the right thing happens.  Needs only minor
>         PyBindGen modifications.  But I think it's a good idea. 


Provided you do not have to use mangled python method names, I am fine
with this.

Mathieu
>