SharedPtr improvements

1. Moves all logic from `SharedImpl` to `SharedObj` (previously split between the two). 2. Moves all methods into the header so that the compiler can inline them. 3. Adds `operator=(T*)` to avoid the overhead of a temporary `SharedImpl`. 4. Adds C++ unit tests. 5. Removes useless methods: move constructors, value constructors, and move assignments.
nschonni · Nov 23, 2018 · a8bc4d6 · a8bc4d6
1 parent 2494f0e
commit a8bc4d6
Show file tree

Hide file tree

Showing 4 changed files with 299 additions and 229 deletions.
diff --git a/src/memory/SharedPtr.cpp b/src/memory/SharedPtr.cpp
@@ -30,85 +30,4 @@ namespace Sass {
  #endif
 
  bool SharedObj::taint = false;
-
- SharedObj::SharedObj()
- : detached(false)
- #ifdef DEBUG_SHARED_PTR
- , dbg(false)
- #endif
- {
- refcounter = 0;
- #ifdef DEBUG_SHARED_PTR
- if (taint) all.push_back(this);
- #endif
- };
-
- SharedObj::~SharedObj() {
- #ifdef DEBUG_SHARED_PTR
- if (dbg) std::cerr << "Destruct " << this << "\n";
- if(!all.empty()) { // check needed for MSVC (no clue why?)
- all.erase(std::remove(all.begin(), all.end(), this), all.end());
- }
- #endif
- };
-
- void SharedPtr::decRefCount() {
- if (node) {
- -- node->refcounter;
- #ifdef DEBUG_SHARED_PTR
- if (node->dbg) std::cerr << "- " << node << " X " << node->refcounter << " (" << this << ") " << "\n";
- #endif
- if (node->refcounter == 0) {
- #ifdef DEBUG_SHARED_PTR
- // AST_Node_Ptr ast = dynamic_cast<AST_Node*>(node);
- if (node->dbg) std::cerr << "DELETE NODE " << node << "\n";
- #endif
- if (!node->detached) {
- delete(node);
- }
- }
- }
- }
-
- void SharedPtr::incRefCount() {
- if (node) {
- ++ node->refcounter;
- node->detached = false;
- #ifdef DEBUG_SHARED_PTR
- if (node->dbg) {
- std::cerr << "+ " << node << " X " << node->refcounter << " (" << this << ") " << "\n";
- }
- #endif
- }
- }
-
- SharedPtr::~SharedPtr() {
- decRefCount();
- }
-
-
- // the create constructor
- SharedPtr::SharedPtr(SharedObj* ptr)
- : node(ptr) {
- incRefCount();
- }
- // copy assignment operator
- SharedPtr& SharedPtr::operator=(const SharedPtr& rhs) {
- void* cur_ptr = (void*) node;
- void* rhs_ptr = (void*) rhs.node;
- if (cur_ptr == rhs_ptr) {
- return *this;
- }
- decRefCount();
- node = rhs.node;
- incRefCount();
- return *this;
- }
-
- // the copy constructor
- SharedPtr::SharedPtr(const SharedPtr& obj)
- : node(obj.node) {
- incRefCount();
- }
-
-}
+}
diff --git a/src/memory/SharedPtr.hpp b/src/memory/SharedPtr.hpp
@@ -3,6 +3,8 @@
 
 #include "sass/base.h"
 
+#include <iostream>
+#include <string>
 #include <vector>
 
 namespace Sass {
@@ -39,177 +41,148 @@ namespace Sass {
  #endif
 
  class SharedObj {
- protected:
- friend class SharedPtr;
- friend class Memory_Manager;
+ public:
+ SharedObj() : refcount(0), detached(false) {
+ #ifdef DEBUG_SHARED_PTR
+ if (taint) all.push_back(this);
+ #endif
+ }
+ virtual ~SharedObj() {
+ #ifdef DEBUG_SHARED_PTR
+ all.clear();
+ #endif
+ }
+
  #ifdef DEBUG_SHARED_PTR
- static std::vector<SharedObj*> all;
- std::string file;
- size_t line;
+ static void dumpMemLeaks();
+ SharedObj* trace(std::string file, size_t line) {
+ this->file = file;
+ this->line = line;
+ return this;
+ }
+ std::string getDbgFile() { return file; }
+ size_t getDbgLine() { return line; }
+ void setDbg(bool dbg) { this->dbg = dbg; }
+ size_t getRefCount() const { return refcount; }
  #endif
- static bool taint;
- long refcounter;
- // long refcount;
+
+ static void setTaint(bool val) { taint = val; }
+
+ virtual const std::string to_string() const = 0;
+ protected:
+ friend class SharedPtr;
+ friend class Memory_Manager;
+ size_t refcount;
  bool detached;
+ static bool taint;
  #ifdef DEBUG_SHARED_PTR
- bool dbg;
- #endif
- public:
- #ifdef DEBUG_SHARED_PTR
- static void dumpMemLeaks();
- SharedObj* trace(std::string file, size_t line) {
- this->file = file;
- this->line = line;
- return this;
- }
+ std::string file;
+ size_t line;
+ bool dbg = false;
+ static std::vector<SharedObj*> all;
  #endif
- SharedObj();
- #ifdef DEBUG_SHARED_PTR
- std::string getDbgFile() {
- return file;
- }
- size_t getDbgLine() {
- return line;
- }
- void setDbg(bool dbg) {
- this->dbg = dbg;
+ };
+
+ class SharedPtr {
+ public:
+ SharedPtr() : node(nullptr) {}
+ SharedPtr(SharedObj* ptr) : node(ptr) {
+ incRefCount();
+ }
+ SharedPtr(const SharedPtr& obj) : SharedPtr(obj.node) {}
+ ~SharedPtr() {
+ decRefCount();
+ }
+
+ SharedPtr& operator=(SharedObj* other_node) {
+ if (node != other_node) {
+ decRefCount();
+ node = other_node;
+ incRefCount();
+ } else if (node != nullptr) {
+ node->detached = false;
  }
- #endif
- static void setTaint(bool val) {
- taint = val;
+ return *this;
  }
 
- virtual const std::string to_string() const = 0;
+ SharedPtr& operator=(const SharedPtr& obj) {
+ return *this = obj.node;
+ }
 
- virtual ~SharedObj();
- long getRefCount() {
- return refcounter;
+ // Prevents all SharedPtrs from freeing this node until it is assigned to another SharedPtr.
+ SharedObj* detach() {
+ if (node != nullptr) node->detached = true;
+ return node;
  }
- };
 
+ SharedObj* obj() const { return node; }
+ SharedObj* operator->() const { return node; }
+ bool isNull() const { return node == nullptr; }
+ operator bool() const { return node != nullptr; }
 
- class SharedPtr {
- protected:
+ protected:
  SharedObj* node;
- protected:
- void decRefCount();
- void incRefCount();
- public:
- // the empty constructor
- SharedPtr()
- : node(NULL) {};
- // the create constructor
- SharedPtr(SharedObj* ptr);
- // the copy constructor
- SharedPtr(const SharedPtr& obj);
- // the move constructor
- SharedPtr(SharedPtr&& obj);
- // copy assignment operator
- SharedPtr& operator=(const SharedPtr& obj);
- // move assignment operator
- SharedPtr& operator=(SharedPtr&& obj);
- // pure virtual destructor
- virtual ~SharedPtr() = 0;
- public:
- SharedObj* obj () const {
- return node;
- };
- SharedObj* operator-> () const {
- return node;
- };
- bool isNull () {
- return node == NULL;
- };
- bool isNull () const {
- return node == NULL;
- };
- SharedObj* detach() const {
- if (node) {
- node->detached = true;
+ void decRefCount() {
+ if (node == nullptr) return;
+ --node->refcount;
+ #ifdef DEBUG_SHARED_PTR
+ if (node->dbg) std::cerr << "- " << node << " X " << node->refcount << " (" << this << ") " << "\n";
+ #endif
+ if (node->refcount == 0 && !node->detached) {
+ #ifdef DEBUG_SHARED_PTR
+ if (node->dbg) std::cerr << "DELETE NODE " << node << "\n";
+ #endif
+ delete node;
  }
- return node;
- };
- operator bool() const {
- return node != NULL;
- };
-
+ }
+ void incRefCount() {
+ if (node == nullptr) return;
+ node->detached = false;
+ ++node->refcount;
+ #ifdef DEBUG_SHARED_PTR
+ if (node->dbg) std::cerr << "+ " << node << " X " << node->refcount << " (" << this << ") " << "\n";
+ #endif
+ }
  };
 
- template < class T >
+ template <class T>
  class SharedImpl : private SharedPtr {
- public:
- SharedImpl()
- : SharedPtr(NULL) {};
- SharedImpl(T* node)
- : SharedPtr(node) {};
- template < class U >
- SharedImpl(SharedImpl<U> obj)
- : SharedPtr(static_cast<T*>(obj.ptr())) {}
- SharedImpl(T&& node)
- : SharedPtr(node) {};
- SharedImpl(const T& node)
- : SharedPtr(node) {};
- // the copy constructor
- SharedImpl(const SharedImpl<T>& impl)
- : SharedPtr(impl.node) {};
- // the move constructor
- SharedImpl(SharedImpl<T>&& impl)
- : SharedPtr(impl.node) {};
- // copy assignment operator
- SharedImpl<T>& operator=(const SharedImpl<T>& rhs) {
- if (node) decRefCount();
- node = rhs.node;
- incRefCount();
- return *this;
+ public:
+ SharedImpl() : SharedPtr(nullptr) {}
+
+ template <class U>
+ SharedImpl(U* node) :
+ SharedPtr(static_cast<T*>(node)) {}
+
+ template <class U>
+ SharedImpl(const SharedImpl<U>& impl) :
+ SharedImpl(impl.ptr()) {}
+
+ template <class U>
+ SharedImpl<T>& operator=(U *rhs) {
+ return static_cast<SharedImpl<T>&>(
+ SharedPtr::operator=(static_cast<T*>(rhs)));
  }
- // move assignment operator
- SharedImpl<T>& operator=(SharedImpl<T>&& rhs) {
- // don't move our self
- if (this != &rhs) {
- if (node) decRefCount();
- node = std::move(rhs.node);
- rhs.node = NULL;
- }
- return *this;
+
+ template <class U>
+ SharedImpl<T>& operator=(const SharedImpl<U>& rhs) {
+ return static_cast<SharedImpl<T>&>(
+ SharedPtr::operator=(static_cast<const SharedImpl<T>&>(rhs)));
  }
 
- // allow implicit conversion to string
- // relies on base class implementation
  operator const std::string() const {
  if (node) return node->to_string();
- else return std::string("[NULLPTR]");
+ return "[nullptr]";
  }
 
- ~SharedImpl() {};
- public:
- operator T*() const {
- return static_cast<T*>(this->obj());
- }
- operator T&() const {
- return *static_cast<T*>(this->obj());
- }
- T& operator* () const {
- return *static_cast<T*>(this->obj());
- };
- T* operator-> () const {
- return static_cast<T*>(this->obj());
- };
- T* ptr () const {
- return static_cast<T*>(this->obj());
- };
- T* detach() const {
- if (this->obj() == NULL) return NULL;
- return static_cast<T*>(SharedPtr::detach());
- }
- bool isNull() const {
- return this->obj() == NULL;
- }
- bool operator<(const T& rhs) const {
- return *this->ptr() < rhs;
- };
- operator bool() const {
- return this->obj() != NULL;
- };
+ using SharedPtr::isNull;
+ using SharedPtr::operator bool;
+ operator T*() const { return static_cast<T*>(this->obj()); }
+ operator T&() const { return *static_cast<T*>(this->obj()); }
+ T& operator* () const { return *static_cast<T*>(this->obj()); };
+ T* operator-> () const { return static_cast<T*>(this->obj()); };
+ T* ptr () const { return static_cast<T*>(this->obj()); };
+ T* detach() { return static_cast<T*>(SharedPtr::detach()); }
  };
 
 }