From a364e85e1dec5285b0350134274c3e32e080ec6d Mon Sep 17 00:00:00 2001 From: Atlante45 Date: Wed, 25 Nov 2015 17:21:56 -0800 Subject: [PATCH] Remove DeleteHooks --- .../src/octree/OctreeQueryNode.cpp | 2 - .../src/octree/OctreeSendThread.h | 1 - libraries/entities/src/EntityTree.cpp | 7 +--- libraries/entities/src/EntityTreeElement.cpp | 23 +--------- libraries/octree/src/OctreeElement.cpp | 33 --------------- libraries/octree/src/OctreeElement.h | 22 ++-------- libraries/octree/src/OctreeElementBag.cpp | 42 ++++--------------- libraries/octree/src/OctreeElementBag.h | 12 ++---- 8 files changed, 18 insertions(+), 124 deletions(-) diff --git a/assignment-client/src/octree/OctreeQueryNode.cpp b/assignment-client/src/octree/OctreeQueryNode.cpp index f70ff62f91..10f1d2ec14 100644 --- a/assignment-client/src/octree/OctreeQueryNode.cpp +++ b/assignment-client/src/octree/OctreeQueryNode.cpp @@ -59,7 +59,6 @@ OctreeQueryNode::~OctreeQueryNode() { void OctreeQueryNode::nodeKilled() { _isShuttingDown = true; - elementBag.unhookNotifications(); // if our node is shutting down, then we no longer need octree element notifications if (_octreeSendThread) { // just tell our thread we want to shutdown, this is asynchronous, and fast, we don't need or want it to block // while the thread actually shuts down @@ -69,7 +68,6 @@ void OctreeQueryNode::nodeKilled() { void OctreeQueryNode::forceNodeShutdown() { _isShuttingDown = true; - elementBag.unhookNotifications(); // if our node is shutting down, then we no longer need octree element notifications if (_octreeSendThread) { // we really need to force our thread to shutdown, this is synchronous, we will block while the thread actually // shuts down because we really need it to shutdown, and it's ok if we wait for it to complete diff --git a/assignment-client/src/octree/OctreeSendThread.h b/assignment-client/src/octree/OctreeSendThread.h index 6e640942e7..6775e56820 100644 --- a/assignment-client/src/octree/OctreeSendThread.h +++ b/assignment-client/src/octree/OctreeSendThread.h @@ -17,7 +17,6 @@ #include #include -#include #include "OctreeQueryNode.h" diff --git a/libraries/entities/src/EntityTree.cpp b/libraries/entities/src/EntityTree.cpp index 9eac14e4b4..5ac892de94 100644 --- a/libraries/entities/src/EntityTree.cpp +++ b/libraries/entities/src/EntityTree.cpp @@ -44,12 +44,7 @@ void EntityTree::createRootElement() { } OctreeElementPointer EntityTree::createNewElement(unsigned char* octalCode) { - EntityTreeElementPointer newElement = EntityTreeElementPointer(new EntityTreeElement(octalCode), - // see comment int EntityTreeElement::createNewElement - [=](EntityTreeElement* dyingElement) { - EntityTreeElementPointer tmpSharedPointer(dyingElement); - dyingElement->notifyDeleteHooks(); - }); + auto newElement = EntityTreeElementPointer(new EntityTreeElement(octalCode)); newElement->setTree(std::static_pointer_cast(shared_from_this())); return std::static_pointer_cast(newElement); } diff --git a/libraries/entities/src/EntityTreeElement.cpp b/libraries/entities/src/EntityTreeElement.cpp index 02552ef488..8403db8b2f 100644 --- a/libraries/entities/src/EntityTreeElement.cpp +++ b/libraries/entities/src/EntityTreeElement.cpp @@ -28,29 +28,8 @@ EntityTreeElement::~EntityTreeElement() { _octreeMemoryUsage -= sizeof(EntityTreeElement); } -// This will be called primarily on addChildAt(), which means we're adding a child of our -// own type to our own tree. This means we should initialize that child with any tree and type -// specific settings that our children must have. OctreeElementPointer EntityTreeElement::createNewElement(unsigned char* octalCode) { - EntityTreeElementPointer newChild = - EntityTreeElementPointer(new EntityTreeElement(octalCode), - // This is a little bit horrible, but I haven't found a better way. The OctreeElement - // destructor used to call notifyDeleteHooks(), which calls zero or more of - // OctreeElementDeleteHook::elementDeleted - // which (now) expects an OctreeElementPointer argument. The destructor doesn't have - // access to the shared pointer (which has had its reference count drop to zero, - // or the destructor wouldn't have been called). The destructor also can't - // make a new shared pointer -- shared_from_this() is forbidden in a destructor, and - // using OctreeElementPointer(this) also fails. So, I've installed a custom deleter: - [=](EntityTreeElement* dyingElement) { - // make a new shared pointer with a reference count of 1 (and no custom deleter) - EntityTreeElementPointer tmpSharedPointer(dyingElement); - // call notifyDeleteHooks which will use shared_from_this() to get this same - // shared pointer, for use with the elementDeleted calls. - dyingElement->notifyDeleteHooks(); - // And now tmpSharedPointer's reference count drops to zero and the - // normal destructors are called. - }); + auto newChild = EntityTreeElementPointer(new EntityTreeElement(octalCode)); newChild->setTree(_myTree); return newChild; } diff --git a/libraries/octree/src/OctreeElement.cpp b/libraries/octree/src/OctreeElement.cpp index 1313dc499d..f16e1dc88d 100644 --- a/libraries/octree/src/OctreeElement.cpp +++ b/libraries/octree/src/OctreeElement.cpp @@ -95,10 +95,6 @@ void OctreeElement::init(unsigned char * octalCode) { } OctreeElement::~OctreeElement() { - // We can't call notifyDeleteHooks from here: - // notifyDeleteHooks(); - // see comment in EntityTreeElement::createNewElement. - assert(_deleteHooksNotified); _voxelNodeCount--; if (isLeaf()) { _voxelNodeLeafCount--; @@ -521,35 +517,6 @@ float OctreeElement::distanceToPoint(const glm::vec3& point) const { return distance; } -QReadWriteLock OctreeElement::_deleteHooksLock; -std::vector OctreeElement::_deleteHooks; - -void OctreeElement::addDeleteHook(OctreeElementDeleteHook* hook) { - _deleteHooksLock.lockForWrite(); - _deleteHooks.push_back(hook); - _deleteHooksLock.unlock(); -} - -void OctreeElement::removeDeleteHook(OctreeElementDeleteHook* hook) { - _deleteHooksLock.lockForWrite(); - for (unsigned int i = 0; i < _deleteHooks.size(); i++) { - if (_deleteHooks[i] == hook) { - _deleteHooks.erase(_deleteHooks.begin() + i); - break; - } - } - _deleteHooksLock.unlock(); -} - -void OctreeElement::notifyDeleteHooks() { - _deleteHooksLock.lockForRead(); - for (unsigned int i = 0; i < _deleteHooks.size(); i++) { - _deleteHooks[i]->elementDeleted(shared_from_this()); - } - _deleteHooksLock.unlock(); - _deleteHooksNotified = true; -} - bool OctreeElement::findSpherePenetration(const glm::vec3& center, float radius, glm::vec3& penetration, void** penetratedObject) const { // center and radius are in meters, so we have to scale the _cube into world-frame diff --git a/libraries/octree/src/OctreeElement.h b/libraries/octree/src/OctreeElement.h index ed92e8a193..c686970c3a 100644 --- a/libraries/octree/src/OctreeElement.h +++ b/libraries/octree/src/OctreeElement.h @@ -33,20 +33,15 @@ class EncodeBitstreamParams; class Octree; class OctreeElement; class OctreeElementBag; -class OctreeElementDeleteHook; class OctreePacketData; class ReadBitstreamToTreeParams; class Shape; class VoxelSystem; -typedef std::shared_ptr OctreeElementPointer; -typedef std::shared_ptr ConstOctreeElementPointer; -typedef std::shared_ptr OctreePointer; -// Callers who want delete hook callbacks should implement this class -class OctreeElementDeleteHook { -public: - virtual void elementDeleted(OctreeElementPointer element) = 0; -}; +using OctreeElementPointer = std::shared_ptr; +using OctreeElementWeakPointer = std::weak_ptr; +using ConstOctreeElementPointer = std::shared_ptr; +using OctreePointer = std::shared_ptr; class OctreeElement: public std::enable_shared_from_this { @@ -174,9 +169,6 @@ public: bool matchesSourceUUID(const QUuid& sourceUUID) const; static uint16_t getSourceNodeUUIDKey(const QUuid& sourceUUID); - static void addDeleteHook(OctreeElementDeleteHook* hook); - static void removeDeleteHook(OctreeElementDeleteHook* hook); - static void resetPopulationStatistics(); static unsigned long getNodeCount() { return _voxelNodeCount; } static unsigned long getInternalNodeCount() { return _voxelNodeCount - _voxelNodeLeafCount; } @@ -234,7 +226,6 @@ protected: void setChildAtIndex(int childIndex, OctreeElementPointer child); void calculateAACube(); - void notifyDeleteHooks(); AACube _cube; /// Client and server, axis aligned box for bounds of this voxel, 48 bytes @@ -276,11 +267,6 @@ protected: _unknownBufferIndex : 1, _childrenExternal : 1; /// Client only, is this voxel's VBO buffer the unknown buffer index, 1 bit - bool _deleteHooksNotified = false; - - static QReadWriteLock _deleteHooksLock; - static std::vector _deleteHooks; - static AtomicUIntStat _voxelNodeCount; static AtomicUIntStat _voxelNodeLeafCount; diff --git a/libraries/octree/src/OctreeElementBag.cpp b/libraries/octree/src/OctreeElementBag.cpp index 5af63c7bb1..167c3560d6 100644 --- a/libraries/octree/src/OctreeElementBag.cpp +++ b/libraries/octree/src/OctreeElementBag.cpp @@ -12,54 +12,30 @@ #include "OctreeElementBag.h" #include -OctreeElementBag::OctreeElementBag() : - _bagElements() -{ - OctreeElement::addDeleteHook(this); - _hooked = true; -} - -OctreeElementBag::~OctreeElementBag() { - unhookNotifications(); - deleteAll(); -} - -void OctreeElementBag::unhookNotifications() { - if (_hooked) { - OctreeElement::removeDeleteHook(this); - _hooked = false; - } -} - -void OctreeElementBag::elementDeleted(OctreeElementPointer element) { - remove(element); // note: remove can safely handle nodes that aren't in it, so we don't need to check contains() -} - - void OctreeElementBag::deleteAll() { _bagElements.clear(); } - void OctreeElementBag::insert(OctreeElementPointer element) { - _bagElements.insert(element); + _bagElements.insert(element.get(), element); } OctreeElementPointer OctreeElementBag::extract() { - OctreeElementPointer result = NULL; + OctreeElementPointer result; - if (_bagElements.size() > 0) { - QSet::iterator front = _bagElements.begin(); - result = *front; - _bagElements.erase(front); + // Find the first element still alive + while (!_bagElements.empty() && !result) { + auto it = _bagElements.begin(); + result = it->lock(); + _bagElements.erase(it); } return result; } bool OctreeElementBag::contains(OctreeElementPointer element) { - return _bagElements.contains(element); + return _bagElements.contains(element.get()); } void OctreeElementBag::remove(OctreeElementPointer element) { - _bagElements.remove(element); + _bagElements.remove(element.get()); } diff --git a/libraries/octree/src/OctreeElementBag.h b/libraries/octree/src/OctreeElementBag.h index 8ef01b44a2..b5f4d5dcf5 100644 --- a/libraries/octree/src/OctreeElementBag.h +++ b/libraries/octree/src/OctreeElementBag.h @@ -17,12 +17,10 @@ #define hifi_OctreeElementBag_h #include "OctreeElement.h" +#include -class OctreeElementBag : public OctreeElementDeleteHook { - +class OctreeElementBag { public: - OctreeElementBag(); - ~OctreeElementBag(); void insert(OctreeElementPointer element); // put a element into the bag OctreeElementPointer extract(); // pull a element out of the bag (could come in any order) @@ -32,13 +30,9 @@ public: int count() const { return _bagElements.size(); } void deleteAll(); - virtual void elementDeleted(OctreeElementPointer element); - - void unhookNotifications(); private: - QSet _bagElements; - bool _hooked; + QHash _bagElements; }; typedef QMap OctreeElementExtraEncodeData;