From 3be38322647de269bf6f74c2a5a81501f168939c Mon Sep 17 00:00:00 2001 From: Andreas Stöckel Date: Mon, 3 Nov 2014 00:33:30 +0000 Subject: implemented equals operator, fixed assignment operator, NodeManager now asserts that all nodes have been deleted in its destructor git-svn-id: file:///var/local/svn/basicwriter@93 daaaf23c-2e50-4459-9457-1e69db5a47bf --- src/core/dom/Node.cpp | 62 +++++++++++++++++---------- src/core/dom/Node.hpp | 115 ++++++++++++++++++++++++++++++++++++++------------ 2 files changed, 126 insertions(+), 51 deletions(-) (limited to 'src/core/dom') diff --git a/src/core/dom/Node.cpp b/src/core/dom/Node.cpp index ad114bf..dcf4261 100644 --- a/src/core/dom/Node.cpp +++ b/src/core/dom/Node.cpp @@ -17,8 +17,6 @@ */ #include -#include - #include #include "Node.hpp" @@ -151,10 +149,12 @@ NodeManager::~NodeManager() // Perform a final sweep sweep(); - // Free all nodes managed by the node manager + // All nodes should have been deleted! + assert(nodes.empty()); + + // Free all nodes managed by the node manager (we'll get here if assertions + // are disabled) if (!nodes.empty()) { - std::cout << "[NodeManager] Warning: " << nodes.size() - << " nodes have not been deleted!" << std::endl; ScopedIncrement incr{deletionRecursionDepth}; for (auto &e : nodes) { delete e.first; @@ -162,25 +162,36 @@ NodeManager::~NodeManager() } } -NodeDescriptor *NodeManager::getDescriptor(Node *n, bool create) +NodeDescriptor *NodeManager::getDescriptor(Node *n) { if (n) { auto it = nodes.find(n); if (it != nodes.end()) { return &(it->second); - } else if (create) { - return &(nodes.emplace(std::make_pair(n, NodeDescriptor{})) - .first->second); } } return nullptr; } +void NodeManager::registerNode(Node *n) +{ + nodes.emplace(std::make_pair(n, NodeDescriptor{})); +} + void NodeManager::addRef(Node *tar, Node *src) { - getDescriptor(tar, true)->incrNodeDegree(RefDir::in, src); + // Fetch the node descriptors for the two nodes + NodeDescriptor *dTar = getDescriptor(tar); + NodeDescriptor *dSrc = getDescriptor(src); + + // Store the tar <- src reference + assert(dTar); + dTar->incrNodeDegree(RefDir::in, src); + if (src) { - getDescriptor(src, true)->incrNodeDegree(RefDir::out, tar); + // Store the src -> tar reference + assert(dSrc); + dSrc->incrNodeDegree(RefDir::out, tar); } else { // We have just added a root reference, remove the element from the // list of marked nodes @@ -188,11 +199,11 @@ void NodeManager::addRef(Node *tar, Node *src) } } -void NodeManager::delRef(Node *tar, Node *src, bool all) +void NodeManager::deleteRef(Node *tar, Node *src, bool all) { // Fetch the node descriptors for the two nodes - NodeDescriptor *dTar = getDescriptor(tar, false); - NodeDescriptor *dSrc = getDescriptor(src, false); + NodeDescriptor *dTar = getDescriptor(tar); + NodeDescriptor *dSrc = getDescriptor(src); // Decrement the output degree of the source node first if (dSrc) { @@ -205,11 +216,11 @@ void NodeManager::delRef(Node *tar, Node *src, bool all) // if it has no root reference, add it to the "marked" set which is // subject to tracing garbage collection if (dTar->refInCount() == 0) { - delNode(tar, dTar); + deleteNode(tar, dTar); } else if (dTar->rootRefCount == 0) { // Call the tracing garbage collector if the number of marked nodes // is larger than the threshold value and this function was not - // called from inside the delNode function + // called from inside the deleteNode function marked.insert(tar); if (marked.size() >= threshold) { sweep(); @@ -218,9 +229,10 @@ void NodeManager::delRef(Node *tar, Node *src, bool all) } } -void NodeManager::delNode(Node *n, NodeDescriptor *descr) +void NodeManager::deleteNode(Node *n, NodeDescriptor *descr) { - // Increment the recursion depth counter. The "delRef" function called below + // Increment the recursion depth counter. The "deleteRef" function called + // below // may descend further into this function and the actual deletion should be // done in a single step. { @@ -231,8 +243,11 @@ void NodeManager::delNode(Node *n, NodeDescriptor *descr) // Remove all output references of this node while (!descr->refOut.empty()) { - delRef(descr->refOut.begin()->first, n, true); + deleteRef(descr->refOut.begin()->first, n, true); } + + // Remove the node from the "marked" set + marked.erase(n); } purgeDeleted(); @@ -265,6 +280,7 @@ void NodeManager::sweep() // Increment the deletionRecursionDepth counter to prevent deletion // of nodes while sweep is running ScopedIncrement incr{deletionRecursionDepth}; + // Fetch the next node in the "marked" list and remove it Node *curNode = *(marked.begin()); @@ -280,7 +296,7 @@ void NodeManager::sweep() marked.erase(curNode); // Fetch the node descriptor - NodeDescriptor *descr = getDescriptor(curNode, false); + NodeDescriptor *descr = getDescriptor(curNode); if (!descr) { continue; } @@ -296,8 +312,8 @@ void NodeManager::sweep() for (auto &src : descr->refIn) { Node *srcNode = src.first; - // Abort if the node is nullptr or already in the reachable - // list + // Abort if the node already in the reachable list, + // otherwise add the node to the queue if it was not visited if (reachable.find(srcNode) != reachable.end()) { isReachable = true; break; @@ -316,7 +332,7 @@ void NodeManager::sweep() } } else { for (auto n : visited) { - delNode(n, getDescriptor(n, false)); + deleteNode(n, getDescriptor(n)); } } } diff --git a/src/core/dom/Node.hpp b/src/core/dom/Node.hpp index a564868..ce1e7f1 100644 --- a/src/core/dom/Node.hpp +++ b/src/core/dom/Node.hpp @@ -172,7 +172,7 @@ protected: * Creates it if it does not exist and the "create" parameter is set to * true. */ - NodeDescriptor *getDescriptor(Node *n, bool create); + NodeDescriptor *getDescriptor(Node *n); /** * Purges the nodes in the "deleted" set. @@ -186,10 +186,10 @@ protected: * @param n is the node that should be deleted. * @param */ - void delNode(Node *n, NodeDescriptor *descr); + void deleteNode(Node *n, NodeDescriptor *descr); /** - * Internal version of the delRef function with an additional "all" + * Internal version of the deleteRef function with an additional "all" * parameter. Removes a reference to the given target node from the source * node. * @@ -202,7 +202,7 @@ protected: * given node is deleted and all references to it should be purged, no * matter what. */ - void delRef(Node *tar, Node *src, bool all); + void deleteRef(Node *tar, Node *src, bool all); public: NodeManager() : threshold(NODE_MANAGER_SWEEP_THRESHOLD) {} @@ -214,6 +214,14 @@ public: */ ~NodeManager(); + /** + * Registers a node for being used with the NodeManager. + * + * @param n is the node which is registered for being used with the + * NodeManager. + */ + void registerNode(Node *n); + /** * Stores a reference to the given target node from the given source node. * If the source pointer is set to nullptr, this means that the target node @@ -235,7 +243,7 @@ public: * @param src is the source node from which the target node was referenced * or nullptr if the target node is referenced from the local scope. */ - void delRef(Node *tar, Node *src) { delRef(tar, src, false); } + void deleteRef(Node *tar, Node *src) { deleteRef(tar, src, false); } /** * Performs garbage collection. @@ -243,13 +251,13 @@ public: void sweep(); }; -template +template class BaseHandle; -template +template class RootedHandle; -template +template class Handle; /** @@ -263,27 +271,29 @@ protected: NodeManager &mgr; public: - Node(NodeManager &mgr) : mgr(mgr){}; + Node(NodeManager &mgr) : mgr(mgr) { mgr.registerNode(this); }; virtual ~Node(){}; NodeManager &getManager() { return mgr; } template - Handle acquire(const BaseHandle &h) { + Handle acquire(const BaseHandle &h) + { return Handle(h, this); } template - Handle acquire(BaseHandle &&h) { + Handle acquire(BaseHandle &&h) + { return Handle(h, this); } template - Handle acquire(T *t) { + Handle acquire(T *t) + { return Handle(t, this); } - }; template @@ -300,7 +310,6 @@ protected: T *ptr; public: - /** * Constructor of the base handle class. * @@ -317,6 +326,27 @@ public: * Provides access to the underlying node. */ T &operator*() { return *ptr; } + + /** + * Comparison operator between base handle and base handle. + */ + bool operator==(const BaseHandle &h) const { return ptr == h.ptr; } + + /** + * Comparison operator between base handle and pointer. + */ + friend bool operator==(const BaseHandle &h, const Node *n) + { + return h.ptr == n; + } + + /** + * Comparison operator between base handle and pointer. + */ + friend bool operator==(const Node *n, const BaseHandle &h) + { + return h.ptr == n; + } }; /** @@ -326,7 +356,6 @@ public: */ template class RootedHandle : public BaseHandle { - private: void addRef() { @@ -336,11 +365,11 @@ private: } } - void delRef() + void deleteRef() { if (BaseHandle::ptr) { - BaseHandle::ptr->getManager().delRef(BaseHandle::ptr, - nullptr); + BaseHandle::ptr->getManager().deleteRef(BaseHandle::ptr, + nullptr); } } @@ -368,6 +397,34 @@ public: h.ptr = nullptr; } + /** + * Assignment operator. Assigns the given handle to this handle instance. + * Both handles are indistinguishable after the operation. + * + * @param h is the handle that should be asigned to this instance. + */ + RootedHandle &operator=(const RootedHandle &h) + { + deleteRef(); + this->ptr = h.ptr; + addRef(); + return *this; + } + + /** + * Move assignment operator. Moves the given rvalue handle into this + * instance. + * + * @param h is the handle to be moved to this instance. + */ + RootedHandle &operator=(RootedHandle &&h) + { + deleteRef(); + this->ptr = h.ptr; + h.ptr = nullptr; + return *this; + } + /** * Assignment operator. Assigns the given handle to this handle instance. * Both handles are indistinguishable after the operation. @@ -376,7 +433,7 @@ public: */ RootedHandle &operator=(const BaseHandle &h) { - delRef(); + deleteRef(); this->ptr = h.ptr; addRef(); return *this; @@ -390,7 +447,7 @@ public: */ RootedHandle &operator=(BaseHandle &&h) { - delRef(); + deleteRef(); this->ptr = h.ptr; h.ptr = nullptr; return *this; @@ -414,7 +471,7 @@ public: * Destructor of the RootedHandle class, deletes all refrences the class is * still holding. */ - ~RootedHandle() { delRef(); } + ~RootedHandle() { deleteRef(); } }; /** @@ -434,10 +491,10 @@ private: } } - void delRef() + void deleteRef() { if (BaseHandle::ptr && owner) { - owner->getManager().delRef(BaseHandle::ptr, owner); + owner->getManager().deleteRef(BaseHandle::ptr, owner); } } @@ -478,7 +535,7 @@ public: */ Handle &operator=(const Handle &h) { - delRef(); + deleteRef(); this->ptr = h.ptr; this->owner = h.owner; addRef(); @@ -493,7 +550,7 @@ public: */ Handle &operator=(Handle &&h) { - delRef(); + deleteRef(); this->ptr = h.ptr; this->owner = h.owner; h.ptr = nullptr; @@ -529,8 +586,7 @@ public: * @param owner is the node which owns this handle instance. The ptr node * is guaranteed to live at least as long as the owner. */ - Handle(BaseHandle &&h, Node *owner) - : BaseHandle(h.ptr), owner(owner) + Handle(BaseHandle &&h, Node *owner) : BaseHandle(h.ptr), owner(owner) { h.ptr = nullptr; } @@ -539,9 +595,12 @@ public: * Destructor of the Handle class, deletes all refrences the class is still * holding. */ - ~Handle() { delRef(); } + ~Handle() { deleteRef(); } }; +using RootedNode = RootedHandle; +using NodeHandle = Handle; + } } -- cgit v1.2.3