Bugzilla – Bug 86
add missing functions to Mac48Address code
Last modified: 2008-07-01 13:32:18 EDT
You need to log in before you can comment on or make changes to this bug.
the two patchsets below add 3 new methods to Mac48Address: bool Mac48Address::IsBroadcast (void) const; bool Mac48Address::IsMulticast (void) const; bool operator < (const Mac48Address &a, const Mac48Address &b); http://code.nsnam.org/mathieu/ns-3-wifi/rev/88023edeb6ea http://code.nsnam.org/mathieu/ns-3-wifi/rev/34f117b7a03b May I merge these two patches into the trunk ?
+bool operator < (const Mac48Address &a, const Mac48Address &b) +{ + uint8_t aP[6]; + uint8_t bP[6]; + a.CopyTo (aP); + b.CopyTo (bP); Isn't it easier/faster to compare directly m_data of a and b? I think you can do it if only operator < is a member of the class... (in C++ you can access private members of other instances of the same class!).
(In reply to comment #1) > +bool operator < (const Mac48Address &a, const Mac48Address &b) > +{ > + uint8_t aP[6]; > + uint8_t bP[6]; > + a.CopyTo (aP); > + b.CopyTo (bP); > > Isn't it easier/faster to compare directly m_data of a and b? I think you can > do it if only operator < is a member of the class... (in C++ you can access > private members of other instances of the same class!). you can make the non-member function be a friend of the class. But, clearly, the goal was not performance here. >
(In reply to comment #2) > (In reply to comment #1) > > +bool operator < (const Mac48Address &a, const Mac48Address &b) > > +{ > > + uint8_t aP[6]; > > + uint8_t bP[6]; > > + a.CopyTo (aP); > > + b.CopyTo (bP); > > > > Isn't it easier/faster to compare directly m_data of a and b? I think you can > > do it if only operator < is a member of the class... (in C++ you can access > > private members of other instances of the same class!). > > you can make the non-member function be a friend of the class. But, clearly, > the goal was not performance here. I have no proof of this, but I think "operator <" is going to be performance sensitive because it is used by stl::map and stl::set, for example. I have another idea; why not make Mac48Address store a uint64_t internally? This way comparison could become very fast on 64 bit processors, and probably also even in 32 bit ones (compiler optimisation). That is assuming support for 64 bit integers is portable, of course (I have no idea).
> > you can make the non-member function be a friend of the class. But, clearly, > > the goal was not performance here. I will do the above to avoid the extra copy. > > I have no proof of this, but I think "operator <" is going to be performance > sensitive because it is used by stl::map and stl::set, for example. > > I have another idea; why not make Mac48Address store a uint64_t internally? If we ever get into a performance bottleneck, we will optimize the code and use a uint64_t if it turns out to be really faster. However, until this happens, let's not make the implementation unreadable. > This way comparison could become very fast on 64 bit processors, and probably > also even in 32 bit ones (compiler optimisation). That is assuming support for > 64 bit integers is portable, of course (I have no idea). We already decided that it was portable enough for us since we use it in a lot of places.
OK by me to commit.