HPCC-15084 CInterface atomics

Changes in jiface to use std::atomic for more efficient implementation of
link/release.

Also cleaned up the code to common up the template/non-template versions.

Signed-off-by: Richard Chapman <rchapman@hpccsystems.com>

system/jlib/jiface.cpp

@@ -23,10 +23,6 @@
 
 //===========================================================================
 
-void CInterface::beforeDispose()
-{
-}
-
 //===========================================================================
 
 #if !defined(_WIN32) && !defined(__GNUC__)

system/jlib/jiface.hpp

@@ -23,6 +23,7 @@
 
 #include "platform.h"
 #include <string.h>
+#include <atomic>
 #include "jscm.hpp"
 #include "jatomic.hpp"
 
@@ -61,25 +62,34 @@ void jlib_decl raiseAssertCore(const char *assertion, const char *file, unsigned
 
 #define DEAD_PSEUDO_COUNT               0x3fffffff
 
+class CEmptyClass
+{
+};
+
+template <class INTERFACE>
+class CInterfaceOf;
+
 //The simplest implementation of IInterface.  Can be used in situations where speed is critical, and
 //there will never be a need for the equivalent of beforeDispose().
 //It is generally recommended to use CInterface for general classes since derived classes may need beforeDispose().
-class jlib_decl CSimpleInterface
+template <class INTERFACE>
+class CSimpleInterfaceOf : public INTERFACE
 {
-    friend class CInterface;    // want to keep xxcount private outside this pair of classes
+    friend class CInterfaceOf<INTERFACE>;    // want to keep xxcount private outside this pair of classes
 public:
-    inline virtual ~CSimpleInterface() {}
+    inline virtual ~CSimpleInterfaceOf() {}
 
-    inline CSimpleInterface()           { atomic_set(&xxcount, 1); }
-    inline bool IsShared(void) const    { return atomic_read(&xxcount) > 1; }
-    inline int getLinkCount(void) const { return atomic_read(&xxcount); }
+    inline CSimpleInterfaceOf() : xxcount(1) { }
+    inline bool IsShared(void) const    { return xxcount.load(std::memory_order_relaxed) > 1; }
+    inline int getLinkCount(void) const { return xxcount.load(std::memory_order_relaxed); }
 
-    inline void Link() const            { atomic_inc(&xxcount); }
+    inline void Link() const { xxcount.fetch_add(1,std::memory_order_relaxed); }
 
     inline bool Release(void) const
     {
-        if (atomic_dec_and_test(&xxcount))
+        if (xxcount.fetch_sub(1,std::memory_order_release) == 1)
         {
+            std::atomic_thread_fence(std::memory_order_acquire);  // Because Herb says so.
             delete this;
             return true;
         }
@@ -87,38 +97,49 @@ public:
     }
 
 private:
-    mutable atomic_t xxcount;
+    CSimpleInterfaceOf(const CSimpleInterfaceOf &) : xxcount(1) {};
+    CSimpleInterfaceOf(CSimpleInterfaceOf &&) = delete;
+    CSimpleInterfaceOf & operator = (const CSimpleInterfaceOf &) = delete;
+    mutable std::atomic<unsigned> xxcount;
 };
 
+template class CSimpleInterfaceOf<CEmptyClass>;
+class jlib_decl CSimpleInterface : public CSimpleInterfaceOf<CEmptyClass> {};
+
 // A more general implementation of IInterface that includes a virtual function beforeDispose().
 // beforeDispose() allows an a fully constructed object to be cleaned up (which means that virtual
 // function calls still work (unlike a virtual destructor).
 // It makes it possible to implement a cache which doesn't link count the object, but is cleared when
 // the object is disposed without a critical section in Release().  (See pattern details below).
-class jlib_decl CInterface : public CSimpleInterface
+template <class INTERFACE>
+class CInterfaceOf : public CSimpleInterfaceOf<INTERFACE>
 {
 public:
-    virtual void beforeDispose();
+    virtual void beforeDispose() {}
 
-    inline bool isAlive() const         { return atomic_read(&xxcount) < DEAD_PSEUDO_COUNT; }       //only safe if Link() is called first
+    inline bool isAlive() const         { return this->xxcount.load(std::memory_order_relaxed) < DEAD_PSEUDO_COUNT; }       //only safe if Link() is called first
 
     inline bool Release(void) const
     {
-        if (atomic_dec_and_test(&xxcount))
+        if (this->xxcount.fetch_sub(1,std::memory_order_release) == 1)
         {
-            //Because beforeDispose could cause this object to be linked/released or call isAlive(), xxcount is set
+            unsigned zero = 0;
+            //Because beforeDispose could cause this object to be linked/released or call isAlive(), this->xxcount is set
             //to a a high mid-point positive number to avoid poss. of releasing again.
-            if (atomic_cas(&xxcount, DEAD_PSEUDO_COUNT, 0))
+            if (this->xxcount.compare_exchange_strong(zero, DEAD_PSEUDO_COUNT, std::memory_order_acq_rel))
             {
-                const_cast<CInterface *>(this)->beforeDispose();
+                const_cast<CInterfaceOf<INTERFACE> *>(this)->beforeDispose();
                 delete this;
                 return true;
             }
         }
         return false;
     }
+
 };
 
+class jlib_decl CInterface : public CInterfaceOf<CEmptyClass> {};
+
 //---------------------------------------------------------------------------------------------------------------------
 
 // An implementation of IInterface for objects that are never shared.  Only likely to be used in a few situations.
@@ -145,105 +166,6 @@ public:
 //- Template variants -------------------------------------------------------------------------------------------------
 
 template <class INTERFACE>
-class CInterfaceOf;
-
-template <class INTERFACE>
-class CReusableInterfaceOf;
-
-// A thread safe basic implementation of IInterface that destroys the object when last Release() occurs.
-template <class INTERFACE>
-class CSimpleInterfaceOf : public INTERFACE
-{
-    friend class CInterfaceOf<INTERFACE>;    // want to keep xxcount private outside this pair of classes
-    friend class CReusableInterfaceOf<INTERFACE>;
-public:
-    inline virtual ~CSimpleInterfaceOf() {}
-
-    inline CSimpleInterfaceOf()           { atomic_set(&xxcount, 1); }
-    inline bool IsShared(void) const    { return atomic_read(&xxcount) > 1; }
-    inline int getLinkCount(void) const { return atomic_read(&xxcount); }
-
-    inline void Link() const            { atomic_inc(&xxcount); }
-
-    inline bool Release(void) const
-    {
-        if (atomic_dec_and_test(&xxcount))
-        {
-            delete this;
-            return true;
-        }
-        return false;
-    }
-
-private:
-    mutable atomic_t xxcount;
-};
-
-// A more general implementation of IInterface that includes a virtual function beforeDispose().
-// beforeDispose() allows a fully constructed object to be cleaned up (which means that virtual
-// function calls still work (unlike a virtual destructor).
-// It makes it possible to implement a cache which doesn't link count the object, but is cleared when
-// the object is disposed without a critical section in Release().  (See pattern details below).
-template <class INTERFACE>
-class CInterfaceOf : public CSimpleInterfaceOf<INTERFACE>
-{
-public:
-    virtual void beforeDispose() {}
-
-    inline bool isAlive() const         { return atomic_read(&this->xxcount) < DEAD_PSEUDO_COUNT; }       //only safe if Link() is called first
-
-    inline bool Release(void) const
-    {
-        if (atomic_dec_and_test(&this->xxcount))
-        {
-            //Because beforeDispose could cause this object to be linked/released or call isAlive(), xxcount is set
-            //to a a high mid-point positive number to avoid poss. of releasing again.
-            if (atomic_cas(&this->xxcount, DEAD_PSEUDO_COUNT, 0))
-            {
-                const_cast<CInterfaceOf<INTERFACE> *>(this)->beforeDispose();
-                delete this;
-                return true;
-            }
-        }
-        return false;
-    }
-};
-
-// An extension of CInterfaceOf that allows objects that are being destroyed to be added to a free list instead.
-// Before disposing addToFree() is called.  That function can link the object and return true which will prevent
-// the object being destroyed.
-template <class INTERFACE>
-class CReusableInterfaceOf : public CInterfaceOf<INTERFACE>
-{
-public:
-    //If this function returns true the object is added to a free list - if so it must be linked in the process.
-    //It *must not* link and release the object if it returns false.
-    virtual bool addToFreeList() const { return false; }
-
-    inline bool Release(void) const
-    {
-        if (atomic_dec_and_test(&this->xxcount))
-        {
-            if (addToFreeList())
-            {
-                //It could have been added, reused and disposed between the call and this return, so must return immediately
-                //and in particular cannot check/modify link count
-                return true;
-            }
-
-            //NOTE: This class cannot be used for caches which don't link the pointers => use atomic_set instead of cas
-            atomic_set(&this->xxcount, DEAD_PSEUDO_COUNT);
-            const_cast<CReusableInterfaceOf<INTERFACE> *>(this)->beforeDispose();
-            delete this;
-            return true;
-        }
-        return false;
-    }
-};
-
-//---------------------------------------------------------------------------------------------------------------------
-
-template <class INTERFACE>
 class CSingleThreadInterfaceOf;
 
 //An equivalent to CSimpleInterfaceOf that is not thread safe - but avoids the cost of atomic operations.