avoiding duplicate refcount implementations

View: New views
4 Messages — Rating Filter:   Alert me  

avoiding duplicate refcount implementations

by Mathieu Lacage :: Rate this Message:

Reply to Author | View Threaded | Show Only this Message

hi,

RefCountBase cannot be used by some classes within ns-3 which means
duplicate refcount implementations. I spotted:
  - 2 for performance reasons
  - 5 to avoid multiple inheritance
  - 2 for no apparent good reason

Based on some work in the multithreading simulator code by guillaume, I
added a new template-based refcounting class which avoids both the
performance and the multiple inheritance problems in
src/core/simple-ref-count.h and got rid of all the duplicate
implementations in our codebase. I left the existing RefCountBase class
in src/core/ for compatibility reasons.

For those who wonder about the SimpleRefCount name, I chose this because
I have a couple of other refcount implementations in the multithreading
code (AtomicRefCount, MutexRefCount, and HashTlsRefCount) which I intend
to merge at some later point.

Here is the full patch which does all of this: since it's pretty
mechanical, straightforward, and source-level compatible, I intend to
push it in 2 or 3 days unless I hear complaints about the idea.

Mathieu

[ref-count.patch]

diff -r 5cb9e32ed4bc src/applications/radvd/radvd-interface.h
--- a/src/applications/radvd/radvd-interface.h Mon Nov 09 16:45:54 2009 +0100
+++ b/src/applications/radvd/radvd-interface.h Mon Nov 09 16:52:00 2009 +0100
@@ -22,7 +22,7 @@
 #define RADVD_INTERFACE_H
 
 #include <list>
-
+#include "ns3/simple-ref-count.h"
 #include "radvd-prefix.h"
 
 namespace ns3
@@ -33,7 +33,7 @@
  * \class RadvdInterface
  * \brief Radvd interface configuration.
  */
-class RadvdInterface : public RefCountBase
+class RadvdInterface : public SimpleRefCount<RadvdInterface>
 {
   public:
     /**
diff -r 5cb9e32ed4bc src/applications/radvd/radvd-prefix.h
--- a/src/applications/radvd/radvd-prefix.h Mon Nov 09 16:45:54 2009 +0100
+++ b/src/applications/radvd/radvd-prefix.h Mon Nov 09 16:52:00 2009 +0100
@@ -24,6 +24,7 @@
 #include <stdint.h>
 
 #include "ns3/ipv6-address.h"
+#include "ns3/simple-ref-count.h"
 
 namespace ns3
 {
@@ -33,7 +34,7 @@
  * \class RadvdPrefix
  * \brief Router prefix for radvd application.
  */
-class RadvdPrefix : public RefCountBase
+class RadvdPrefix : public SimpleRefCount<RadvdPrefix>
 {
   public:
     /**
diff -r 5cb9e32ed4bc src/common/ascii-writer.h
--- a/src/common/ascii-writer.h Mon Nov 09 16:45:54 2009 +0100
+++ b/src/common/ascii-writer.h Mon Nov 09 16:52:00 2009 +0100
@@ -23,7 +23,7 @@
 
 #include <stdint.h>
 #include <ostream>
-#include "ns3/ref-count-base.h"
+#include "ns3/simple-ref-count.h"
 #include "ns3/ptr.h"
 
 namespace ns3 {
@@ -35,7 +35,7 @@
  *
  * \brief Ascii output
  */
-class AsciiWriter : public RefCountBase
+class AsciiWriter : public SimpleRefCount<AsciiWriter>
 {
 public:
   static Ptr<AsciiWriter> Get (std::ostream &os);
diff -r 5cb9e32ed4bc src/common/packet.cc
--- a/src/common/packet.cc Mon Nov 09 16:45:54 2009 +0100
+++ b/src/common/packet.cc Mon Nov 09 16:52:00 2009 +0100
@@ -112,21 +112,6 @@
 }
 
 
-void
-Packet::Ref (void) const
-{
-  m_refCount++;
-}
-void
-Packet::Unref (void) const
-{
-  m_refCount--;
-  if (m_refCount == 0)
-    {
-      delete this;
-    }
-}
-
 Ptr<Packet>
 Packet::Copy (void) const
 {
@@ -141,8 +126,7 @@
     m_byteTagList (),
     m_packetTagList (),
     m_metadata (m_globalUid, 0),
-    m_nixVector (0),
-    m_refCount (1)
+    m_nixVector (0)
 {
   m_globalUid++;
 }
@@ -151,8 +135,7 @@
   : m_buffer (o.m_buffer),
     m_byteTagList (o.m_byteTagList),
     m_packetTagList (o.m_packetTagList),
-    m_metadata (o.m_metadata),
-    m_refCount (1)
+    m_metadata (o.m_metadata)
 {
   o.m_nixVector ? m_nixVector = o.m_nixVector->Copy ()
                 : m_nixVector = 0;
@@ -179,8 +162,7 @@
     m_byteTagList (),
     m_packetTagList (),
     m_metadata (m_globalUid, size),
-    m_nixVector (0),
-    m_refCount (1)
+    m_nixVector (0)
 {
   m_globalUid++;
 }
@@ -189,8 +171,7 @@
     m_byteTagList (),
     m_packetTagList (),
     m_metadata (m_globalUid, size),
-    m_nixVector (0),
-    m_refCount (1)
+    m_nixVector (0)
 {
   m_globalUid++;
   m_buffer.AddAtStart (size);
@@ -204,8 +185,7 @@
     m_byteTagList (byteTagList),
     m_packetTagList (packetTagList),
     m_metadata (metadata),
-    m_nixVector (0),
-    m_refCount (1)
+    m_nixVector (0)
 {}
 
 Ptr<Packet>
diff -r 5cb9e32ed4bc src/common/packet.h
--- a/src/common/packet.h Mon Nov 09 16:45:54 2009 +0100
+++ b/src/common/packet.h Mon Nov 09 16:52:00 2009 +0100
@@ -33,6 +33,7 @@
 #include "ns3/assert.h"
 #include "ns3/ptr.h"
 #include "ns3/deprecated.h"
+#include "ns3/simple-ref-count.h"
 
 namespace ns3 {
 
@@ -199,12 +200,9 @@
  * The performance aspects of the Packet API are discussed in
  * \ref packetperf
  */
-class Packet
+class Packet : public SimpleRefCount<Packet>
 {
 public:
-  void Ref (void) const;
-  void Unref (void) const;
-
   Ptr<Packet> Copy (void) const;
 
   /**
@@ -541,7 +539,6 @@
   /* Please see comments above about nix-vector */
   Ptr<NixVector> m_nixVector;
 
-  mutable uint32_t m_refCount;
   static uint32_t m_globalUid;
 };
 
diff -r 5cb9e32ed4bc src/contrib/flow-monitor/flow-classifier.h
--- a/src/contrib/flow-monitor/flow-classifier.h Mon Nov 09 16:45:54 2009 +0100
+++ b/src/contrib/flow-monitor/flow-classifier.h Mon Nov 09 16:52:00 2009 +0100
@@ -21,7 +21,7 @@
 #ifndef __FLOW_CLASSIFIER_H__
 #define __FLOW_CLASSIFIER_H__
 
-#include "ns3/ref-count-base.h"
+#include "ns3/simple-ref-count.h"
 #include <ostream>
 
 namespace ns3 {
@@ -39,7 +39,7 @@
 /// statistics reference only those abstract identifiers in order to
 /// keep the core architecture generic and not tied down to any
 /// particular flow capture method or classification system.
-class FlowClassifier : public RefCountBase
+class FlowClassifier : public SimpleRefCount<FlowClassifier>
 {
   FlowId m_lastNewFlowId;
 
diff -r 5cb9e32ed4bc src/contrib/flow-monitor/flow-probe.h
--- a/src/contrib/flow-monitor/flow-probe.h Mon Nov 09 16:45:54 2009 +0100
+++ b/src/contrib/flow-monitor/flow-probe.h Mon Nov 09 16:52:00 2009 +0100
@@ -24,7 +24,7 @@
 #include <map>
 #include <vector>
 
-#include "ns3/ref-count-base.h"
+#include "ns3/simple-ref-count.h"
 #include "ns3/flow-classifier.h"
 #include "ns3/nstime.h"
 
@@ -36,7 +36,7 @@
 /// in a specific point of the simulated space, report those events to
 /// the global FlowMonitor, and collect its own flow statistics
 /// regarding only the packets that pass through that probe.
-class FlowProbe : public RefCountBase
+class FlowProbe : public SimpleRefCount<FlowProbe>
 {
 protected:
   
diff -r 5cb9e32ed4bc src/core/attribute.h
--- a/src/core/attribute.h Mon Nov 09 16:45:54 2009 +0100
+++ b/src/core/attribute.h Mon Nov 09 16:52:00 2009 +0100
@@ -23,7 +23,7 @@
 #include <string>
 #include <stdint.h>
 #include "ptr.h"
-#include "ref-count-base.h"
+#include "simple-ref-count.h"
 
 namespace ns3 {
 
@@ -48,7 +48,7 @@
  * Most subclasses of this base class are implemented by the
  * ATTRIBUTE_HELPER_* macros.
  */
-class AttributeValue : public RefCountBase
+class AttributeValue : public SimpleRefCount<AttributeValue>
 {
 public:
   AttributeValue ();
@@ -94,7 +94,7 @@
  * of this base class are usually provided through the MakeAccessorHelper
  * template functions, hidden behind an ATTRIBUTE_HELPER_* macro.
  */
-class AttributeAccessor : public RefCountBase
+class AttributeAccessor : public SimpleRefCount<AttributeAccessor>
 {
 public:
   AttributeAccessor ();
@@ -146,7 +146,7 @@
  * Most subclasses of this base class are implemented by the
  * ATTRIBUTE_HELPER_HEADER and ATTRIBUTE_HELPER_CPP macros.
  */
-class AttributeChecker : public RefCountBase
+class AttributeChecker : public SimpleRefCount<AttributeChecker>
 {
 public:
   AttributeChecker ();
diff -r 5cb9e32ed4bc src/core/callback.h
--- a/src/core/callback.h Mon Nov 09 16:45:54 2009 +0100
+++ b/src/core/callback.h Mon Nov 09 16:52:00 2009 +0100
@@ -27,6 +27,7 @@
 #include "type-traits.h"
 #include "attribute.h"
 #include "attribute-helper.h"
+#include "simple-ref-count.h"
 #include <typeinfo>
 
 namespace ns3 {
@@ -72,25 +73,10 @@
   }
 };
 
-class CallbackImplBase
+class CallbackImplBase : public SimpleRefCount<CallbackImplBase>
 {
 public:
-  CallbackImplBase ()
-    : m_count (1) {}
-  virtual ~CallbackImplBase () {}
-  void Ref (void) const {
-    m_count++;
-  }
-  void Unref (void) const {
-    m_count--;
-    if (m_count == 0) {
-      delete this;
-    }
-  }
-  uint32_t GetReferenceCount (void) const { return m_count; }
   virtual bool IsEqual (Ptr<const CallbackImplBase> other) const = 0;
-private:
-  mutable uint32_t m_count;
 };
 
 // declare the CallbackImpl class
diff -r 5cb9e32ed4bc src/core/ref-count-base.cc
--- a/src/core/ref-count-base.cc Mon Nov 09 16:45:54 2009 +0100
+++ b/src/core/ref-count-base.cc Mon Nov 09 16:52:00 2009 +0100
@@ -1,52 +1,8 @@
-/* -*- Mode:C++; c-file-style:"gnu"; indent-tabs-mode:nil; -*- */
-/*
- * Copyright (c) 2007 Georgia Tech Research Corporation
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License version 2 as
- * published by the Free Software Foundation;
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program; if not, write to the Free Software
- * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
- *
- * Author:  George Riley <riley@...>
- * Adapted from original code in object.h by:
- * Authors: Gustavo Carneiro <gjcarneiro@...>,
- *          Mathieu Lacage <mathieu.lacage@...>
- */
-
 #include "ref-count-base.h"
 
 namespace ns3 {
 
-RefCountBase::RefCountBase()
-  : m_count (1)
-{
-}
-
-RefCountBase::RefCountBase (const RefCountBase &o)
- : m_count (1)
+RefCountBase::~RefCountBase ()
 {}
-RefCountBase &
-RefCountBase::operator = (const RefCountBase &o)
-{
-  return *this;
-}
-
-RefCountBase::~RefCountBase ()
-{
-}
-
-uint32_t
-RefCountBase::GetReferenceCount (void) const
-{
-  return m_count;
-}
 
 } // namespace ns3
diff -r 5cb9e32ed4bc src/core/ref-count-base.h
--- a/src/core/ref-count-base.h Mon Nov 09 16:45:54 2009 +0100
+++ b/src/core/ref-count-base.h Mon Nov 09 16:52:00 2009 +0100
@@ -23,7 +23,7 @@
 #ifndef __REF_COUNT_BASE_H__
 #define __REF_COUNT_BASE_H__
 
-#include <stdint.h>
+#include "simple-ref-count.h"
 
 namespace ns3 {
 
@@ -37,60 +37,15 @@
  * class ns3::Object.
  *
  */
-class RefCountBase
+class RefCountBase : public SimpleRefCount<RefCountBase>
 {
 public:
-  RefCountBase();
-  RefCountBase (const RefCountBase &o);
-  RefCountBase &operator = (const RefCountBase &o);
-  virtual ~RefCountBase ();
   /**
-   * Increment the reference count. This method should not be called
-   * by user code. RefCountBase instances are expected to be used in
-   * conjunction with the Ptr template which would make calling Ref
-   * unecessary and dangerous.
+   * This only thing this class does it declare a virtual destructor
    */
-  inline void Ref (void) const;
-  /**
-   * Decrement the reference count. This method should not be called
-   * by user code. RefCountBase instances are expected to be used in
-   * conjunction with the Ptr template which would make calling Ref
-   * unecessary and dangerous.
-   */
-  inline void Unref (void) const;
-
-  /**
-   * Get the reference count of the object.  Normally not needed; for language bindings.
-   */
-  uint32_t GetReferenceCount (void) const;
-
-private:
-  // Note we make this mutable so that the const methods can still
-  // change it.
-  mutable uint32_t m_count;  // Reference count
+  virtual ~RefCountBase () = 0;
 };
 
 } // namespace ns3
 
-namespace ns3 {
-
-// Implementation of the in-line methods
-void
-RefCountBase::Ref (void) const
-{
-  m_count++;
-}
-
-void
-RefCountBase::Unref (void) const
-{
-  m_count--;
-  if (m_count == 0)
-    { // All references removed, ok to delete
-      delete this;
-    }
-}
-
-} // namespace ns3
-
 #endif /* __REF_COUNT_BASE_H__*/
diff -r 5cb9e32ed4bc src/core/simple-ref-count.h
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/src/core/simple-ref-count.h Mon Nov 09 16:52:00 2009 +0100
@@ -0,0 +1,98 @@
+/* -*- Mode:C++; c-file-style:"gnu"; indent-tabs-mode:nil; -*- */
+/*
+ * Copyright (c) 2007 Georgia Tech Research Corporation, INRIA
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation;
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
+ *
+ * Authors: George Riley <riley@...>
+ *          Gustavo Carneiro <gjcarneiro@...>,
+ *          Mathieu Lacage <mathieu.lacage@...>
+ */
+#ifndef SIMPLE_REF_COUNT_H
+#define SIMPLE_REF_COUNT_H
+
+#include "empty.h"
+#include <stdint.h>
+
+namespace ns3 {
+
+/**
+ * \brief A template-based reference counting class
+ *
+ * This template can be used to give reference-counting powers
+ * to a class. This template does not require this class to
+ * have a virtual destructor or no parent class.
+ *
+ * \internal
+ * Note: we rely on the fairly common EBCO (empty base class optimization)
+ * to get rid of the empty parent when the user does not provide
+ * a non-trivial parent.
+ */
+template <typename T, typename PARENT = empty>
+class SimpleRefCount : public PARENT
+{
+public:
+  SimpleRefCount ()
+    : m_count (1)
+  {}
+  SimpleRefCount (const SimpleRefCount &o)
+    : m_count (1)
+  {}
+  SimpleRefCount &operator = (const SimpleRefCount &o)
+  {
+    return *this;
+  }
+  /**
+   * Increment the reference count. This method should not be called
+   * by user code. SimpleRefCount instances are expected to be used in
+   * conjunction with the Ptr template which would make calling Ref
+   * unecessary and dangerous.
+   */
+  inline void Ref (void) const
+  {
+    m_count++;
+  }
+  /**
+   * Decrement the reference count. This method should not be called
+   * by user code. SimpleRefCount instances are expected to be used in
+   * conjunction with the Ptr template which would make calling Ref
+   * unecessary and dangerous.
+   */
+  inline void Unref (void) const
+  {
+    m_count--;
+    if (m_count == 0)
+      {
+ delete static_cast<T*> (const_cast<SimpleRefCount *> (this));
+      }
+  }
+
+  /**
+   * Get the reference count of the object.  
+   * Normally not needed; for language bindings.
+   */
+  uint32_t GetReferenceCount (void) const
+  {
+    return m_count;
+  }
+
+private:
+  // Note we make this mutable so that the const methods can still
+  // change it.
+  mutable uint32_t m_count;
+};
+
+} // namespace ns3
+
+#endif /* SIMPLE_REF_COUNT_H */
diff -r 5cb9e32ed4bc src/core/system-thread.h
--- a/src/core/system-thread.h Mon Nov 09 16:45:54 2009 +0100
+++ b/src/core/system-thread.h Mon Nov 09 16:52:00 2009 +0100
@@ -42,7 +42,7 @@
  *
  * Synchronization between threads is provided via the SystemMutex class.
  */
-class SystemThread
+class SystemThread : public SimpleRefCount<SystemThread>
 {
 public:
   /**
@@ -72,14 +72,6 @@
    * is asking the SystemThread to call object->MyMethod () in a new thread
    * of execution.
    *
-   * Remember that if you are invoking a callback on an object that is
-   * managed by a smart pointer, you need to call PeekPointer.
-   *
-   *   Ptr<MyClass> myPtr = Create<MyClass> ();
-   *   Ptr<SystemThread> st = Create<SystemThread> (
-   *     MakeCallback (&MyClass::MyMethod, PeekPointer (myPtr)));
-   *   st->Start ();
-   *
    * Just like any thread, you can synchronize with its termination.  The
    * method provided to do this is Join (). If you call Join() you will block
    * until the SystemThread run method returns.
@@ -108,22 +100,6 @@
   ~SystemThread();
 
   /**
-   * Increment the reference count. This method should not be called
-   * by user code. Object instances are expected to be used in conjunction
-   * of the Ptr template which would make calling Ref unecessary and
-   * dangerous.
-   */
-  inline void Ref (void) const;
-
-  /**
-   * Decrement the reference count. This method should not be called
-   * by user code. Object instances are expected to be used in conjunction
-   * of the Ptr template which would make calling Ref unecessary and
-   * dangerous.
-   */
-  inline void Unref (void) const;
-
-  /**
    * @brief Start a thread of execution, running the provided callback.
    */
   void Start (void);
@@ -173,26 +149,9 @@
 
 private:
   SystemThreadImpl * m_impl;
-  mutable uint32_t m_count;
   bool m_break;
 };
 
- void
-SystemThread::Ref (void) const
-{
-  m_count++;
-}
-
- void
-SystemThread::Unref (void) const
-{
-  m_count--;
-  if (m_count == 0)
-    {
-      delete this;
-    }
-}
-
 } //namespace ns3
 
 #endif /* SYSTEM_THREAD_H */
diff -r 5cb9e32ed4bc src/core/trace-source-accessor.cc
--- a/src/core/trace-source-accessor.cc Mon Nov 09 16:45:54 2009 +0100
+++ b/src/core/trace-source-accessor.cc Mon Nov 09 16:52:00 2009 +0100
@@ -22,23 +22,8 @@
 namespace ns3 {
 
 TraceSourceAccessor::TraceSourceAccessor ()
-  : m_count (1)
 {}
 TraceSourceAccessor::~TraceSourceAccessor ()
 {}
-void
-TraceSourceAccessor::Ref (void) const
-{
-  m_count++;
-}
-void
-TraceSourceAccessor::Unref (void) const
-{
-  m_count--;
-  if (m_count == 0)
-    {
-      delete this;
-    }
-}
 
 } // namespace ns3
diff -r 5cb9e32ed4bc src/core/trace-source-accessor.h
--- a/src/core/trace-source-accessor.h Mon Nov 09 16:45:54 2009 +0100
+++ b/src/core/trace-source-accessor.h Mon Nov 09 16:52:00 2009 +0100
@@ -23,6 +23,7 @@
 #include <stdint.h>
 #include "callback.h"
 #include "ptr.h"
+#include "simple-ref-count.h"
 
 namespace ns3 {
 
@@ -36,13 +37,11 @@
  * This class abstracts the kind of trace source to which we want to connect
  * and provides services to Connect and Disconnect a sink to a trace source.
  */
-class TraceSourceAccessor
+class TraceSourceAccessor : public SimpleRefCount<TraceSourceAccessor>
 {
 public:
   TraceSourceAccessor ();
   virtual ~TraceSourceAccessor ();
-  void Ref (void) const;
-  void Unref (void) const;
 
   /**
    * \param obj the object instance which contains the target trace source.
@@ -66,8 +65,6 @@
    * \param cb the callback to disconnect from the target trace source.
    */
   virtual bool Disconnect (ObjectBase *obj, std::string context, const CallbackBase &cb) const = 0;
-private:
-  mutable uint32_t m_count;
 };
 
 /**
diff -r 5cb9e32ed4bc src/core/unix-system-thread.cc
--- a/src/core/unix-system-thread.cc Mon Nov 09 16:45:54 2009 +0100
+++ b/src/core/unix-system-thread.cc Mon Nov 09 16:52:00 2009 +0100
@@ -141,8 +141,7 @@
 // class above.
 //
 SystemThread::SystemThread (Callback<void> callback)
-  : m_impl (new SystemThreadImpl (callback)),
-    m_count (1)
+  : m_impl (new SystemThreadImpl (callback))
 {
   NS_LOG_FUNCTION_NOARGS ();
 }
diff -r 5cb9e32ed4bc src/core/wscript
--- a/src/core/wscript Mon Nov 09 16:45:54 2009 +0100
+++ b/src/core/wscript Mon Nov 09 16:52:00 2009 +0100
@@ -90,6 +90,7 @@
         'callback.h',
         'object-base.h',
         'ref-count-base.h',
+        'simple-ref-count.h',
         'type-id.h',
         'attribute-list.h',
         'ptr.h',
diff -r 5cb9e32ed4bc src/devices/mesh/dot11s/ie-dot11s-beacon-timing.h
--- a/src/devices/mesh/dot11s/ie-dot11s-beacon-timing.h Mon Nov 09 16:45:54 2009 +0100
+++ b/src/devices/mesh/dot11s/ie-dot11s-beacon-timing.h Mon Nov 09 16:52:00 2009 +0100
@@ -31,7 +31,7 @@
  * \ingroup dot11s
  * \brief Describes one unit of beacon timing element
  */
-class IeBeaconTimingUnit : public RefCountBase
+class IeBeaconTimingUnit : public SimpleRefCount<IeBeaconTimingUnit>
 {
 public:
   IeBeaconTimingUnit ();
diff -r 5cb9e32ed4bc src/devices/mesh/dot11s/ie-dot11s-preq.h
--- a/src/devices/mesh/dot11s/ie-dot11s-preq.h Mon Nov 09 16:45:54 2009 +0100
+++ b/src/devices/mesh/dot11s/ie-dot11s-preq.h Mon Nov 09 16:52:00 2009 +0100
@@ -33,7 +33,7 @@
  * \brief Describes an address unit in PREQ information element
  * See 7.3.2.96 for more details
  */
-class DestinationAddressUnit : public RefCountBase
+class DestinationAddressUnit : public SimpleRefCount<DestinationAddressUnit>
 {
 public:
   DestinationAddressUnit ();
diff -r 5cb9e32ed4bc src/devices/mesh/mesh-wifi-interface-mac-plugin.h
--- a/src/devices/mesh/mesh-wifi-interface-mac-plugin.h Mon Nov 09 16:45:54 2009 +0100
+++ b/src/devices/mesh/mesh-wifi-interface-mac-plugin.h Mon Nov 09 16:52:00 2009 +0100
@@ -25,7 +25,7 @@
 #include "ns3/packet.h"
 #include "ns3/mac48-address.h"
 #include "ns3/mesh-wifi-beacon.h"
-#include "ns3/ref-count-base.h"
+#include "ns3/simple-ref-count.h"
 
 namespace ns3 {
 
@@ -38,7 +38,7 @@
  *
  * TODO: plugins description
  */
-class MeshWifiInterfaceMacPlugin : public RefCountBase
+class MeshWifiInterfaceMacPlugin : public SimpleRefCount<MeshWifiInterfaceMacPlugin>
 {
 public:
   /// This is for subclasses
diff -r 5cb9e32ed4bc src/devices/mesh/wifi-information-element-vector.h
--- a/src/devices/mesh/wifi-information-element-vector.h Mon Nov 09 16:45:54 2009 +0100
+++ b/src/devices/mesh/wifi-information-element-vector.h Mon Nov 09 16:52:00 2009 +0100
@@ -23,6 +23,7 @@
 #define IE_VECTOR_H
 
 #include "ns3/header.h"
+#include "ns3/simple-ref-count.h"
 
 namespace ns3 {
 class Packet;
@@ -79,7 +80,7 @@
  * Element ID as defined in this standard. The Length field specifies the number of octets in the Information
  * field.
  */
-class WifiInformationElement : public RefCountBase
+class WifiInformationElement : public SimpleRefCount<WifiInformationElement>
 {
 public:
   ///\name Each subclass must implement
diff -r 5cb9e32ed4bc src/devices/wifi/interference-helper.h
--- a/src/devices/wifi/interference-helper.h Mon Nov 09 16:45:54 2009 +0100
+++ b/src/devices/wifi/interference-helper.h Mon Nov 09 16:52:00 2009 +0100
@@ -27,7 +27,7 @@
 #include "wifi-preamble.h"
 #include "wifi-phy-standard.h"
 #include "ns3/nstime.h"
-#include "ns3/ref-count-base.h"
+#include "ns3/simple-ref-count.h"
 
 namespace ns3 {
 
@@ -36,7 +36,7 @@
 class InterferenceHelper
 {
 public:
-  class Event : public RefCountBase
+  class Event : public SimpleRefCount<InterferenceHelper::Event>
   {
   public:
     Event (uint32_t size, WifiMode payloadMode,
diff -r 5cb9e32ed4bc src/node/ipv4-route.h
--- a/src/node/ipv4-route.h Mon Nov 09 16:45:54 2009 +0100
+++ b/src/node/ipv4-route.h Mon Nov 09 16:52:00 2009 +0100
@@ -23,7 +23,7 @@
 #include <vector>
 #include <ostream>
 
-#include "ns3/ref-count-base.h"
+#include "ns3/simple-ref-count.h"
 #include "ipv4-address.h"
 
 namespace ns3 {
@@ -38,7 +38,8 @@
  * This is a reference counted object.  In the future, we will add other
  * entries from struct dst_entry, struct rtable, and struct dst_ops as needed.
  */
-class Ipv4Route : public RefCountBase {
+class Ipv4Route : public SimpleRefCount<Ipv4Route>
+{
 public:
   Ipv4Route ();
 
@@ -103,7 +104,8 @@
  *
  * \brief Ipv4 multicast route cache entry (similar to Linux struct mfc_cache)
  */
-class Ipv4MulticastRoute : public RefCountBase {
+class Ipv4MulticastRoute : public SimpleRefCount<Ipv4MulticastRoute>
+{
 public:
   Ipv4MulticastRoute ();
 
diff -r 5cb9e32ed4bc src/node/ipv6-route.h
--- a/src/node/ipv6-route.h Mon Nov 09 16:45:54 2009 +0100
+++ b/src/node/ipv6-route.h Mon Nov 09 16:52:00 2009 +0100
@@ -25,7 +25,7 @@
 #include <vector>
 #include <ostream>
 
-#include "ns3/ref-count-base.h"
+#include "ns3/simple-ref-count.h"
 
 #include "ipv6-address.h"
 
@@ -39,7 +39,7 @@
  * \class Ipv6Route
  * \brief IPv6 route cache entry.
  */
-class Ipv6Route : public RefCountBase
+class Ipv6Route : public SimpleRefCount<Ipv6Route>
 {
   public:
     /**
@@ -129,7 +129,7 @@
  * \class Ipv6MulticastRoute
  * \brief IPv6 multicast route entry.
  */
-class Ipv6MulticastRoute : public RefCountBase
+class Ipv6MulticastRoute : public SimpleRefCount<Ipv6MulticastRoute>
 {
   public:
     /**
diff -r 5cb9e32ed4bc src/node/packetbb.cc
--- a/src/node/packetbb.cc Mon Nov 09 16:45:54 2009 +0100
+++ b/src/node/packetbb.cc Mon Nov 09 16:52:00 2009 +0100
@@ -497,7 +497,6 @@
 
 PbbPacket::PbbPacket (void)
 {
-  m_refCount = 1;
   m_version = VERSION;
   m_hasseqnum = false;
 }
@@ -746,21 +745,6 @@
   m_messageList.clear ();
 }
 
-void
-PbbPacket::Ref (void) const
-{
-  m_refCount++;
-}
-
-void
-PbbPacket::Unref (void) const
-{
-  m_refCount--;
-  if (m_refCount == 0)
-    {
-      delete this;
-    }
-}
 
 TypeId
 PbbPacket::GetTypeId (void)
@@ -947,7 +931,6 @@
 
 PbbMessage::PbbMessage ()
 {
-  m_refCount = 1;
   /* Default to IPv4 */
   m_addrSize = IPV4;
   m_hasOriginatorAddress = false;
@@ -1274,22 +1257,6 @@
   return m_addressBlockList.clear();
 }
 
-void
-PbbMessage::Ref (void) const
-{
-  m_refCount++;
-}
-
-void
-PbbMessage::Unref (void) const
-{
-  m_refCount--;
-  if (m_refCount == 0)
-    {
-      delete this;
-    }
-}
-
 uint32_t
 PbbMessage::GetSerializedSize (void) const
 {
@@ -1699,13 +1666,10 @@
 /* End PbbMessageIpv6 Class */
 
 PbbAddressBlock::PbbAddressBlock ()
-{
-  m_refCount = 1;
-}
+{}
 
 PbbAddressBlock::~PbbAddressBlock ()
-{
-}
+{}
 
 /* Manipulating the address block */
 
@@ -2002,23 +1966,6 @@
 {
   m_addressTlvList.Clear();
 }
-
-void
-PbbAddressBlock::Ref (void) const
-{
-  m_refCount++;
-}
-
-void
-PbbAddressBlock::Unref (void) const
-{
-  m_refCount--;
-  if (m_refCount == 0)
-    {
-      delete this;
-    }
-}
-
 uint32_t
 PbbAddressBlock::GetSerializedSize (void) const
 {
@@ -2448,7 +2395,6 @@
 
 PbbTlv::PbbTlv (void)
 {
-  m_refCount = 1;
   m_hasTypeExt = false;
   m_hasIndexStart = false;
   m_hasIndexStop = false;
@@ -2573,22 +2519,6 @@
   return m_hasValue;
 }
 
-void
-PbbTlv::Ref (void) const
-{
-  m_refCount++;
-}
-
-void
-PbbTlv::Unref (void) const
-{
-  m_refCount--;
-  if (m_refCount == 0)
-    {
-      delete this;
-    }
-}
-
 uint32_t
 PbbTlv::GetSerializedSize (void) const
 {
diff -r 5cb9e32ed4bc src/node/packetbb.h
--- a/src/node/packetbb.h Mon Nov 09 16:45:54 2009 +0100
+++ b/src/node/packetbb.h Mon Nov 09 16:52:00 2009 +0100
@@ -31,6 +31,7 @@
 #include "ns3/address.h"
 #include "ns3/header.h"
 #include "ns3/buffer.h"
+#include "ns3/simple-ref-count.h"
 
 namespace ns3 {
 
@@ -360,7 +361,7 @@
  *
  * See: http://tools.ietf.org/html/rfc5444 for details.
  */
-class PbbPacket : public Header
+class PbbPacket : public SimpleRefCount<PbbPacket,Header>
 {
 public:
   typedef std::list< Ptr<PbbTlv> >::iterator TlvIterator;
@@ -595,10 +596,6 @@
    */
   void MessageClear (void);
 
-  /* Smart pointer methods */
-  void Ref (void) const;
-  void Unref (void) const;
-
   /* Methods implemented by all headers */
   static TypeId GetTypeId (void);
   virtual TypeId GetInstanceTypeId (void) const;
@@ -644,8 +641,6 @@
 
   bool m_hasseqnum;
   uint16_t m_seqnum;
-
-  mutable uint32_t m_refCount;
 };
 
 /**
@@ -655,7 +650,7 @@
  * virtual base class, when creating a message, you should instantiate either
  * PbbMessageIpv4 or PbbMessageIpv6.
  */
-class PbbMessage
+class PbbMessage : public SimpleRefCount<PbbMessage>
 {
 public:
   typedef std::list< Ptr<PbbTlv> >::iterator TlvIterator;
@@ -959,10 +954,6 @@
    */
   void AddressBlockClear (void);
 
-  /* Smart pointer methods */
-  void Ref (void) const;
-  void Unref (void) const;
-
   /**
    * \brief Deserializes a message, returning the correct object depending on
    *        whether it is an IPv4 message or an IPv6 message.
@@ -1048,8 +1039,6 @@
 
   bool m_hasSequenceNumber;
   uint16_t m_sequenceNumber;
-
-  mutable uint32_t m_refCount;
 };
 
 /**
@@ -1098,7 +1087,7 @@
  * This is a pure virtual base class, when creating address blocks, you should
  * instantiate either PbbAddressBlockIpv4 or PbbAddressBlockIpv6.
  */
-class PbbAddressBlock
+class PbbAddressBlock : public SimpleRefCount<PbbAddressBlock>
 {
 public:
   typedef std::list< Address >::iterator AddressIterator;
@@ -1412,10 +1401,6 @@
    */
   void TlvClear (void);
 
-  /* Smart pointer methods */
-  void Ref (void) const;
-  void Unref (void) const;
-
   /**
    * \return The size (in bytes) needed to serialize this address block.
    */
@@ -1475,8 +1460,6 @@
   std::list<Address> m_addressList;
   std::list<uint8_t> m_prefixList;
   PbbAddressTlvBlock m_addressTlvList;
-
-  mutable uint32_t m_refCount;
 };
 
 /**
@@ -1520,7 +1503,7 @@
 /**
  * \brief A packet or message TLV
  */
-class PbbTlv
+class PbbTlv : public SimpleRefCount<PbbTlv>
 {
 public:
   PbbTlv (void);
@@ -1599,10 +1582,6 @@
    */
   bool HasValue (void) const;
 
-  /* Smart pointer methods */
-  void Ref (void) const;
-  void Unref (void) const;
-
   /**
    * \return The size (in bytes) needed to serialize this TLV.
    */
@@ -1672,8 +1651,6 @@
   bool m_isMultivalue;
   bool m_hasValue;
   Buffer m_value;
-
-  mutable uint32_t m_refCount;
 };
 
 /**
diff -r 5cb9e32ed4bc src/simulator/event-impl.cc
--- a/src/simulator/event-impl.cc Mon Nov 09 16:45:54 2009 +0100
+++ b/src/simulator/event-impl.cc Mon Nov 09 16:52:00 2009 +0100
@@ -20,15 +20,13 @@
 
 #include "event-impl.h"
 
-
 namespace ns3 {
 
 EventImpl::~EventImpl ()
 {}
 
 EventImpl::EventImpl ()
-  : m_cancel (false),
-    m_count (1)
+  : m_cancel (false)
 {}
 
 void
diff -r 5cb9e32ed4bc src/simulator/event-impl.h
--- a/src/simulator/event-impl.h Mon Nov 09 16:45:54 2009 +0100
+++ b/src/simulator/event-impl.h Mon Nov 09 16:52:00 2009 +0100
@@ -21,6 +21,7 @@
 #define EVENT_IMPL_H
 
 #include <stdint.h>
+#include "ns3/simple-ref-count.h"
 
 namespace ns3 {
 
@@ -35,12 +36,10 @@
  * most subclasses are usually created by one of the many Simulator::Schedule
  * methods.
  */
-class EventImpl
+class EventImpl : public SimpleRefCount<EventImpl>
 {
 public:
   EventImpl ();
-  inline void Ref (void) const;
-  inline void Unref (void) const;
   virtual ~EventImpl () = 0;
   /**
    * Called by the simulation engine to notify the event that it has expired.
@@ -64,30 +63,8 @@
 
 private:
   bool m_cancel;
-  mutable uint32_t m_count;
 };
 
-}; // namespace ns3
-
-namespace ns3 {
-
-void
-EventImpl::Ref (void) const
-{
-  m_count++;
-}
-
-void
-EventImpl::Unref (void) const
-{
-  uint32_t c;
-  c = --m_count;
-  if (c == 0)
-    {
-      delete this;
-    }
-}
-
 } // namespace ns3
 
 #endif /* EVENT_IMPL_H */


Re: avoiding duplicate refcount implementations

by Tom Henderson-2 :: Rate this Message:

Reply to Author | View Threaded | Show Only this Message

Mathieu Lacage wrote:

> hi,
>
> RefCountBase cannot be used by some classes within ns-3 which means
> duplicate refcount implementations. I spotted:
>   - 2 for performance reasons
>   - 5 to avoid multiple inheritance
>   - 2 for no apparent good reason
>
> Based on some work in the multithreading simulator code by guillaume, I
> added a new template-based refcounting class which avoids both the
> performance and the multiple inheritance problems in
> src/core/simple-ref-count.h and got rid of all the duplicate
> implementations in our codebase. I left the existing RefCountBase class
> in src/core/ for compatibility reasons.

Do you want to add a doxygen comment in class RefCountBase that it is
deprecated, and direct readers to the new class?

Otherwise, looks fine to me.

(by the way, I think packet.h can safely remove deprecated.h inclusion)


Re: avoiding duplicate refcount implementations

by Mathieu Lacage :: Rate this Message:

Reply to Author | View Threaded | Show Only this Message

On Mon, 2009-11-09 at 22:07 -0800, Tom Henderson wrote:

> Do you want to add a doxygen comment in class RefCountBase that it is
> deprecated, and direct readers to the new class?

I did not plan to make it deprecated but, if it is felt that it should
be killed, I can mark it so in the doxygen.

> Otherwise, looks fine to me.
>
> (by the way, I think packet.h can safely remove deprecated.h inclusion)

Right: I pushed this as a separate patch. Thanks for pointing it out.

Mathieu


Re: avoiding duplicate refcount implementations

by Tom Henderson-2 :: Rate this Message:

Reply to Author | View Threaded | Show Only this Message

Mathieu Lacage wrote:
> On Mon, 2009-11-09 at 22:07 -0800, Tom Henderson wrote:
>
>> Do you want to add a doxygen comment in class RefCountBase that it is
>> deprecated, and direct readers to the new class?
>
> I did not plan to make it deprecated but, if it is felt that it should
> be killed, I can mark it so in the doxygen.

I meant that if you are going to the trouble to change all of the
existing instances, that you leave a comment for new users that they
should consider the new class before using RefCountBase, which is being
retained for backward compatibility reasons.

- Tom