Skip to content

Commit a8bc4d6

Browse files
glebmxzyfer
glebm
authored and
xzyfer
committed
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.
1 parent 2494f0e commit a8bc4d6

File tree

4 files changed

+299
-229
lines changed

4 files changed

+299
-229
lines changed

‎src/memory/SharedPtr.cpp

Lines changed: 1 addition & 82 deletions
Original file line numberDiff line numberDiff line change
@@ -30,85 +30,4 @@ namespace Sass {
3030
#endif
3131

3232
bool SharedObj::taint = false;
33-
34-
SharedObj::SharedObj()
35-
: detached(false)
36-
#ifdef DEBUG_SHARED_PTR
37-
, dbg(false)
38-
#endif
39-
{
40-
refcounter = 0;
41-
#ifdef DEBUG_SHARED_PTR
42-
if (taint) all.push_back(this);
43-
#endif
44-
};
45-
46-
SharedObj::~SharedObj() {
47-
#ifdef DEBUG_SHARED_PTR
48-
if (dbg) std::cerr << "Destruct " << this << "\n";
49-
if(!all.empty()) { // check needed for MSVC (no clue why?)
50-
all.erase(std::remove(all.begin(), all.end(), this), all.end());
51-
}
52-
#endif
53-
};
54-
55-
void SharedPtr::decRefCount() {
56-
if (node) {
57-
-- node->refcounter;
58-
#ifdef DEBUG_SHARED_PTR
59-
if (node->dbg) std::cerr << "- " << node << " X " << node->refcounter << " (" << this << ") " << "\n";
60-
#endif
61-
if (node->refcounter == 0) {
62-
#ifdef DEBUG_SHARED_PTR
63-
// AST_Node_Ptr ast = dynamic_cast<AST_Node*>(node);
64-
if (node->dbg) std::cerr << "DELETE NODE " << node << "\n";
65-
#endif
66-
if (!node->detached) {
67-
delete(node);
68-
}
69-
}
70-
}
71-
}
72-
73-
void SharedPtr::incRefCount() {
74-
if (node) {
75-
++ node->refcounter;
76-
node->detached = false;
77-
#ifdef DEBUG_SHARED_PTR
78-
if (node->dbg) {
79-
std::cerr << "+ " << node << " X " << node->refcounter << " (" << this << ") " << "\n";
80-
}
81-
#endif
82-
}
83-
}
84-
85-
SharedPtr::~SharedPtr() {
86-
decRefCount();
87-
}
88-
89-
90-
// the create constructor
91-
SharedPtr::SharedPtr(SharedObj* ptr)
92-
: node(ptr) {
93-
incRefCount();
94-
}
95-
// copy assignment operator
96-
SharedPtr& SharedPtr::operator=(const SharedPtr& rhs) {
97-
void* cur_ptr = (void*) node;
98-
void* rhs_ptr = (void*) rhs.node;
99-
if (cur_ptr == rhs_ptr) {
100-
return *this;
101-
}
102-
decRefCount();
103-
node = rhs.node;
104-
incRefCount();
105-
return *this;
106-
}
107-
108-
// the copy constructor
109-
SharedPtr::SharedPtr(const SharedPtr& obj)
110-
: node(obj.node) {
111-
incRefCount();
112-
}
113-
114-
}
33+
}

‎src/memory/SharedPtr.hpp

Lines changed: 120 additions & 147 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@
33

44
#include "sass/base.h"
55

6+
#include <iostream>
7+
#include <string>
68
#include <vector>
79

810
namespace Sass {
@@ -39,177 +41,148 @@ namespace Sass {
3941
#endif
4042

4143
class SharedObj {
42-
protected:
43-
friend class SharedPtr;
44-
friend class Memory_Manager;
44+
public:
45+
SharedObj() : refcount(0), detached(false) {
46+
#ifdef DEBUG_SHARED_PTR
47+
if (taint) all.push_back(this);
48+
#endif
49+
}
50+
virtual ~SharedObj() {
51+
#ifdef DEBUG_SHARED_PTR
52+
all.clear();
53+
#endif
54+
}
55+
4556
#ifdef DEBUG_SHARED_PTR
46-
static std::vector<SharedObj*> all;
47-
std::string file;
48-
size_t line;
57+
static void dumpMemLeaks();
58+
SharedObj* trace(std::string file, size_t line) {
59+
this->file = file;
60+
this->line = line;
61+
return this;
62+
}
63+
std::string getDbgFile() { return file; }
64+
size_t getDbgLine() { return line; }
65+
void setDbg(bool dbg) { this->dbg = dbg; }
66+
size_t getRefCount() const { return refcount; }
4967
#endif
50-
static bool taint;
51-
long refcounter;
52-
// long refcount;
68+
69+
static void setTaint(bool val) { taint = val; }
70+
71+
virtual const std::string to_string() const = 0;
72+
protected:
73+
friend class SharedPtr;
74+
friend class Memory_Manager;
75+
size_t refcount;
5376
bool detached;
77+
static bool taint;
5478
#ifdef DEBUG_SHARED_PTR
55-
bool dbg;
56-
#endif
57-
public:
58-
#ifdef DEBUG_SHARED_PTR
59-
static void dumpMemLeaks();
60-
SharedObj* trace(std::string file, size_t line) {
61-
this->file = file;
62-
this->line = line;
63-
return this;
64-
}
79+
std::string file;
80+
size_t line;
81+
bool dbg = false;
82+
static std::vector<SharedObj*> all;
6583
#endif
66-
SharedObj();
67-
#ifdef DEBUG_SHARED_PTR
68-
std::string getDbgFile() {
69-
return file;
70-
}
71-
size_t getDbgLine() {
72-
return line;
73-
}
74-
void setDbg(bool dbg) {
75-
this->dbg = dbg;
84+
};
85+
86+
class SharedPtr {
87+
public:
88+
SharedPtr() : node(nullptr) {}
89+
SharedPtr(SharedObj* ptr) : node(ptr) {
90+
incRefCount();
91+
}
92+
SharedPtr(const SharedPtr& obj) : SharedPtr(obj.node) {}
93+
~SharedPtr() {
94+
decRefCount();
95+
}
96+
97+
SharedPtr& operator=(SharedObj* other_node) {
98+
if (node != other_node) {
99+
decRefCount();
100+
node = other_node;
101+
incRefCount();
102+
} else if (node != nullptr) {
103+
node->detached = false;
76104
}
77-
#endif
78-
static void setTaint(bool val) {
79-
taint = val;
105+
return *this;
80106
}
81107

82-
virtual const std::string to_string() const = 0;
108+
SharedPtr& operator=(const SharedPtr& obj) {
109+
return *this = obj.node;
110+
}
83111

84-
virtual ~SharedObj();
85-
long getRefCount() {
86-
return refcounter;
112+
// Prevents all SharedPtrs from freeing this node until it is assigned to another SharedPtr.
113+
SharedObj* detach() {
114+
if (node != nullptr) node->detached = true;
115+
return node;
87116
}
88-
};
89117

118+
SharedObj* obj() const { return node; }
119+
SharedObj* operator->() const { return node; }
120+
bool isNull() const { return node == nullptr; }
121+
operator bool() const { return node != nullptr; }
90122

91-
class SharedPtr {
92-
protected:
123+
protected:
93124
SharedObj* node;
94-
protected:
95-
void decRefCount();
96-
void incRefCount();
97-
public:
98-
// the empty constructor
99-
SharedPtr()
100-
: node(NULL) {};
101-
// the create constructor
102-
SharedPtr(SharedObj* ptr);
103-
// the copy constructor
104-
SharedPtr(const SharedPtr& obj);
105-
// the move constructor
106-
SharedPtr(SharedPtr&& obj);
107-
// copy assignment operator
108-
SharedPtr& operator=(const SharedPtr& obj);
109-
// move assignment operator
110-
SharedPtr& operator=(SharedPtr&& obj);
111-
// pure virtual destructor
112-
virtual ~SharedPtr() = 0;
113-
public:
114-
SharedObj* obj () const {
115-
return node;
116-
};
117-
SharedObj* operator-> () const {
118-
return node;
119-
};
120-
bool isNull () {
121-
return node == NULL;
122-
};
123-
bool isNull () const {
124-
return node == NULL;
125-
};
126-
SharedObj* detach() const {
127-
if (node) {
128-
node->detached = true;
125+
void decRefCount() {
126+
if (node == nullptr) return;
127+
--node->refcount;
128+
#ifdef DEBUG_SHARED_PTR
129+
if (node->dbg) std::cerr << "- " << node << " X " << node->refcount << " (" << this << ") " << "\n";
130+
#endif
131+
if (node->refcount == 0 && !node->detached) {
132+
#ifdef DEBUG_SHARED_PTR
133+
if (node->dbg) std::cerr << "DELETE NODE " << node << "\n";
134+
#endif
135+
delete node;
129136
}
130-
return node;
131-
};
132-
operator bool() const {
133-
return node != NULL;
134-
};
135-
137+
}
138+
void incRefCount() {
139+
if (node == nullptr) return;
140+
node->detached = false;
141+
++node->refcount;
142+
#ifdef DEBUG_SHARED_PTR
143+
if (node->dbg) std::cerr << "+ " << node << " X " << node->refcount << " (" << this << ") " << "\n";
144+
#endif
145+
}
136146
};
137147

138-
template < class T >
148+
template <class T>
139149
class SharedImpl : private SharedPtr {
140-
public:
141-
SharedImpl()
142-
: SharedPtr(NULL) {};
143-
SharedImpl(T* node)
144-
: SharedPtr(node) {};
145-
template < class U >
146-
SharedImpl(SharedImpl<U> obj)
147-
: SharedPtr(static_cast<T*>(obj.ptr())) {}
148-
SharedImpl(T&& node)
149-
: SharedPtr(node) {};
150-
SharedImpl(const T& node)
151-
: SharedPtr(node) {};
152-
// the copy constructor
153-
SharedImpl(const SharedImpl<T>& impl)
154-
: SharedPtr(impl.node) {};
155-
// the move constructor
156-
SharedImpl(SharedImpl<T>&& impl)
157-
: SharedPtr(impl.node) {};
158-
// copy assignment operator
159-
SharedImpl<T>& operator=(const SharedImpl<T>& rhs) {
160-
if (node) decRefCount();
161-
node = rhs.node;
162-
incRefCount();
163-
return *this;
150+
public:
151+
SharedImpl() : SharedPtr(nullptr) {}
152+
153+
template <class U>
154+
SharedImpl(U* node) :
155+
SharedPtr(static_cast<T*>(node)) {}
156+
157+
template <class U>
158+
SharedImpl(const SharedImpl<U>& impl) :
159+
SharedImpl(impl.ptr()) {}
160+
161+
template <class U>
162+
SharedImpl<T>& operator=(U *rhs) {
163+
return static_cast<SharedImpl<T>&>(
164+
SharedPtr::operator=(static_cast<T*>(rhs)));
164165
}
165-
// move assignment operator
166-
SharedImpl<T>& operator=(SharedImpl<T>&& rhs) {
167-
// don't move our self
168-
if (this != &rhs) {
169-
if (node) decRefCount();
170-
node = std::move(rhs.node);
171-
rhs.node = NULL;
172-
}
173-
return *this;
166+
167+
template <class U>
168+
SharedImpl<T>& operator=(const SharedImpl<U>& rhs) {
169+
return static_cast<SharedImpl<T>&>(
170+
SharedPtr::operator=(static_cast<const SharedImpl<T>&>(rhs)));
174171
}
175172

176-
// allow implicit conversion to string
177-
// relies on base class implementation
178173
operator const std::string() const {
179174
if (node) return node->to_string();
180-
else return std::string("[NULLPTR]");
175+
return "[nullptr]";
181176
}
182177

183-
~SharedImpl() {};
184-
public:
185-
operator T*() const {
186-
return static_cast<T*>(this->obj());
187-
}
188-
operator T&() const {
189-
return *static_cast<T*>(this->obj());
190-
}
191-
T& operator* () const {
192-
return *static_cast<T*>(this->obj());
193-
};
194-
T* operator-> () const {
195-
return static_cast<T*>(this->obj());
196-
};
197-
T* ptr () const {
198-
return static_cast<T*>(this->obj());
199-
};
200-
T* detach() const {
201-
if (this->obj() == NULL) return NULL;
202-
return static_cast<T*>(SharedPtr::detach());
203-
}
204-
bool isNull() const {
205-
return this->obj() == NULL;
206-
}
207-
bool operator<(const T& rhs) const {
208-
return *this->ptr() < rhs;
209-
};
210-
operator bool() const {
211-
return this->obj() != NULL;
212-
};
178+
using SharedPtr::isNull;
179+
using SharedPtr::operator bool;
180+
operator T*() const { return static_cast<T*>(this->obj()); }
181+
operator T&() const { return *static_cast<T*>(this->obj()); }
182+
T& operator* () const { return *static_cast<T*>(this->obj()); };
183+
T* operator-> () const { return static_cast<T*>(this->obj()); };
184+
T* ptr () const { return static_cast<T*>(this->obj()); };
185+
T* detach() { return static_cast<T*>(SharedPtr::detach()); }
213186
};
214187

215188
}

0 commit comments

Comments
 (0)