Browse Source

Refactored Event:
- it no longer uses a shared pointer for every connection which was causing a significant overhead
- it no longer delays disconnected connection cleanup which was causing hundreds of connections to pile up and cause multi-second hangs (coupled with the issue that they all had shared_ptrs to release)

Marko Pintera 10 years ago
parent
commit
3a6ccf391c

+ 1 - 0
BansheeCore/Source/BsQueryManager.cpp

@@ -2,6 +2,7 @@
 #include "BsEventQuery.h"
 #include "BsEventQuery.h"
 #include "BsTimerQuery.h"
 #include "BsTimerQuery.h"
 #include "BsOcclusionQuery.h"
 #include "BsOcclusionQuery.h"
+#include "BsProfilerCPU.h"
 
 
 namespace BansheeEngine
 namespace BansheeEngine
 {
 {

+ 1 - 2
BansheeD3D11RenderSystem/Source/BsD3D11HardwareBuffer.cpp

@@ -161,14 +161,13 @@ namespace BansheeEngine
 			D3D11_MAPPED_SUBRESOURCE mappedSubResource;
 			D3D11_MAPPED_SUBRESOURCE mappedSubResource;
 			mappedSubResource.pData = NULL;
 			mappedSubResource.pData = NULL;
 			mDevice.clearErrors();
 			mDevice.clearErrors();
-			gProfilerCPU().beginSample("Map");
+
 			HRESULT hr = mDevice.getImmediateContext()->Map(mD3DBuffer, 0, mapType, 0, &mappedSubResource);
 			HRESULT hr = mDevice.getImmediateContext()->Map(mD3DBuffer, 0, mapType, 0, &mappedSubResource);
 			if (FAILED(hr) || mDevice.hasError())
 			if (FAILED(hr) || mDevice.hasError())
 			{
 			{
 				String msg = mDevice.getErrorDescription();
 				String msg = mDevice.getErrorDescription();
 				BS_EXCEPT(RenderingAPIException, "Error calling Map: " + msg);
 				BS_EXCEPT(RenderingAPIException, "Error calling Map: " + msg);
 			}
 			}
-			gProfilerCPU().endSample("Map");
 
 
 			pRet = static_cast<void*>(static_cast<char*>(mappedSubResource.pData) + offset);
 			pRet = static_cast<void*>(static_cast<char*>(mappedSubResource.pData) + offset);
 
 

+ 0 - 10
BansheeEngine/Source/BsProfilerOverlay.cpp

@@ -562,11 +562,6 @@ namespace BansheeEngine
 
 
 	void ProfilerOverlayInternal::update()
 	void ProfilerOverlayInternal::update()
 	{
 	{
-		static float pausedTime = 0.0f; // DEBUG ONLY
-
-		if ((gTime().getTime() - pausedTime) <= 5.0f)
-			return;
-
 		const ProfilerReport& latestSimReport = ProfilingManager::instance().getReport(ProfiledThread::Sim);
 		const ProfilerReport& latestSimReport = ProfilingManager::instance().getReport(ProfiledThread::Sim);
 		const ProfilerReport& latestCoreReport = ProfilingManager::instance().getReport(ProfiledThread::Core);
 		const ProfilerReport& latestCoreReport = ProfilingManager::instance().getReport(ProfiledThread::Core);
 
 
@@ -579,11 +574,6 @@ namespace BansheeEngine
 		{
 		{
 			updateGPUSampleContents(ProfilerGPU::instance().getNextReport());
 			updateGPUSampleContents(ProfilerGPU::instance().getNextReport());
 		}
 		}
-
-		if (gTime().getFrameDelta() > 0.100f)
-		{
-			pausedTime = gTime().getTime();
-		}
 	}
 	}
 
 
 	void ProfilerOverlayInternal::targetResized()
 	void ProfilerOverlayInternal::targetResized()

+ 175 - 99
BansheeUtility/Include/BsEvent.h

@@ -11,7 +11,119 @@ namespace BansheeEngine
 	class BaseConnectionData
 	class BaseConnectionData
 	{
 	{
 	public:
 	public:
-		bool isValid;
+		BaseConnectionData()
+			:prev(nullptr), next(nullptr), isActive(true),
+			hasHandleLink(true)
+		{
+			
+		}
+
+		virtual ~BaseConnectionData()
+		{
+			assert(!hasHandleLink && !isActive);
+		}
+
+		virtual void deactivate()
+		{
+			isActive = false;
+		}
+
+		BaseConnectionData* prev;
+		BaseConnectionData* next;
+		bool isActive;
+		bool hasHandleLink;
+	};
+
+	struct EventInternalData
+	{
+		EventInternalData()
+			:mConnections(nullptr), mFreeConnections(nullptr)
+		{ }
+
+		~EventInternalData()
+		{
+			BaseConnectionData* conn = mConnections;
+			while (conn != nullptr)
+			{
+				BaseConnectionData* next = conn->next;
+				bs_delete(conn);
+
+				conn = next;
+			}
+
+			conn = mFreeConnections;
+			while (conn != nullptr)
+			{
+				BaseConnectionData* next = conn->next;
+				bs_delete(conn);
+
+				conn = next;
+			}
+		}
+
+		void disconnect(BaseConnectionData* conn)
+		{
+			BS_LOCK_RECURSIVE_MUTEX(mMutex);
+
+			conn->deactivate();
+			conn->hasHandleLink = false;
+
+			free(conn);
+		}
+
+		void clear()
+		{
+			BS_LOCK_RECURSIVE_MUTEX(mMutex);
+
+			BaseConnectionData* conn = mConnections;
+			while (conn != nullptr)
+			{
+				BaseConnectionData* next = conn->next;
+				conn->deactivate();
+
+				if (!conn->hasHandleLink)
+					free(conn);
+
+				conn = next;
+			}
+		}
+
+		void freeHandle(BaseConnectionData* conn)
+		{
+			BS_LOCK_RECURSIVE_MUTEX(mMutex);
+
+			conn->hasHandleLink = false;
+
+			if (!conn->isActive)
+				free(conn);
+		}
+
+		void free(BaseConnectionData* conn)
+		{
+			if (conn->prev != nullptr)
+				conn->prev->next = conn->next;
+			else
+				mConnections = conn->next;
+
+			if (conn->next != nullptr)
+				conn->next->prev = conn->prev;
+
+			conn->prev = nullptr;
+			conn->next = nullptr;
+
+			if (mFreeConnections != nullptr)
+			{
+				conn->next = mFreeConnections;
+				mFreeConnections->prev = conn;
+			}
+
+			mFreeConnections = conn;
+		}
+
+		BaseConnectionData* mConnections;
+		BaseConnectionData* mFreeConnections;
+
+		BS_RECURSIVE_MUTEX(mMutex);
 	};
 	};
 
 
 	/**
 	/**
@@ -22,22 +134,29 @@ namespace BansheeEngine
 	{
 	{
 	public:
 	public:
 		HEvent()
 		HEvent()
-			:mDisconnectCallback(nullptr), mConnection(nullptr), mEvent(nullptr)
+			:mConnection(nullptr)
 		{ }
 		{ }
 
 
-		HEvent(std::shared_ptr<BaseConnectionData> connection, void* event, void(*disconnectCallback) (const std::shared_ptr<BaseConnectionData>&, void*))
-			:mConnection(connection), mEvent(event), mDisconnectCallback(disconnectCallback)
+		explicit HEvent(const SPtr<EventInternalData>& eventData, BaseConnectionData* connection)
+			:mConnection(connection), mEventData(eventData)
 		{ }
 		{ }
 
 
+		~HEvent()
+		{
+			if (mConnection != nullptr)
+				mEventData->freeHandle(mConnection);
+		}
+
 		/**
 		/**
 		 * @brief	Disconnect from the event you are subscribed to.
 		 * @brief	Disconnect from the event you are subscribed to.
-		 *
-		 * @note	Caller must ensure the event is still valid.
 		 */
 		 */
 		void disconnect()
 		void disconnect()
 		{
 		{
-			if (mConnection != nullptr && mConnection->isValid)
-				mDisconnectCallback(mConnection, mEvent);
+			if (mConnection != nullptr)
+			{
+				mEventData->disconnect(mConnection);
+				mConnection = nullptr;
+			}
 		}
 		}
 
 
 		struct Bool_struct
 		struct Bool_struct
@@ -53,13 +172,12 @@ namespace BansheeEngine
 		*/
 		*/
 		operator int Bool_struct::*() const
 		operator int Bool_struct::*() const
 		{
 		{
-			return ((mConnection != nullptr && mConnection->isValid) ? &Bool_struct::_Member : 0);
+			return (mConnection != nullptr ? &Bool_struct::_Member : 0);
 		}
 		}
 
 
 	private:
 	private:
-		void(*mDisconnectCallback) (const std::shared_ptr<BaseConnectionData>&, void*);
-		std::shared_ptr<BaseConnectionData> mConnection;
-		void* mEvent;
+		BaseConnectionData* mConnection;
+		SPtr<EventInternalData> mEventData;
 	};	
 	};	
 
 
 	/**
 	/**
@@ -74,27 +192,19 @@ namespace BansheeEngine
 		struct ConnectionData : BaseConnectionData
 		struct ConnectionData : BaseConnectionData
 		{
 		{
 		public:
 		public:
-			ConnectionData(std::function<RetType(Args...)> func)
-				:func(func)
-			{ }
-
-			std::function<RetType(Args...)> func;
-		};
+			void deactivate() override
+			{
+				func = nullptr;
 
 
-		struct InternalData
-		{
-			InternalData()
-				:mHasDisconnectedCallbacks(false)
-			{ }
+				BaseConnectionData::deactivate();
+			}
 
 
-			Vector<std::shared_ptr<ConnectionData>> mConnections;
-			bool mHasDisconnectedCallbacks;
-			BS_RECURSIVE_MUTEX(mMutex);
+			std::function<RetType(Args...)> func;
 		};
 		};
 
 
 	public:
 	public:
 		TEvent()
 		TEvent()
-			:mInternalData(bs_shared_ptr<InternalData>())
+			:mInternalData(bs_shared_ptr<EventInternalData>())
 		{ }
 		{ }
 
 
 		~TEvent()
 		~TEvent()
@@ -108,15 +218,33 @@ namespace BansheeEngine
 		 */
 		 */
 		HEvent connect(std::function<RetType(Args...)> func)
 		HEvent connect(std::function<RetType(Args...)> func)
 		{
 		{
-			std::shared_ptr<ConnectionData> connData = bs_shared_ptr<ConnectionData>(func);
-			connData->isValid = true;
+			BS_LOCK_RECURSIVE_MUTEX(mInternalData->mMutex);
 
 
+			ConnectionData* connData = nullptr;
+			if (mInternalData->mFreeConnections != nullptr)
 			{
 			{
-				BS_LOCK_RECURSIVE_MUTEX(mInternalData->mMutex);
-				mInternalData->mConnections.push_back(connData);
+				connData = static_cast<ConnectionData*>(mInternalData->mFreeConnections);
+				mInternalData->mFreeConnections = connData->next;
+
+				if (connData->next != nullptr)
+					connData->next->prev = nullptr;
+
+				connData->isActive = true;
+				connData->hasHandleLink = true;
 			}
 			}
-			
-			return HEvent(connData, this, &TEvent::disconnectCallback);
+
+			if (connData == nullptr)
+				connData = bs_new<ConnectionData>();
+
+			connData->next = mInternalData->mConnections;
+
+			if (mInternalData->mConnections != nullptr)
+				mInternalData->mConnections->prev = connData;
+
+			mInternalData->mConnections = connData;
+			connData->func = func;
+
+			return HEvent(mInternalData, connData);
 		}
 		}
 
 
 		/**
 		/**
@@ -126,33 +254,22 @@ namespace BansheeEngine
 		{
 		{
 			// Increase ref count to ensure this event data isn't destroyed if one of the callbacks
 			// Increase ref count to ensure this event data isn't destroyed if one of the callbacks
 			// deletes the event itself.
 			// deletes the event itself.
-			std::shared_ptr<InternalData> internalData = mInternalData;
+			std::shared_ptr<EventInternalData> internalData = mInternalData;
 
 
 			BS_LOCK_RECURSIVE_MUTEX(internalData->mMutex);
 			BS_LOCK_RECURSIVE_MUTEX(internalData->mMutex);
 
 
-			// Here is the only place we remove connections, in order to allow disconnect() and clear() to be called
-			// recursively from the notify callbacks
-			if (internalData->mHasDisconnectedCallbacks)
+			// Hidden dependency: If any new connections are made during these callbacks they must be
+			// inserted at the start of the linked list so that we don't trigger them here.
+			ConnectionData* conn = static_cast<ConnectionData*>(mInternalData->mConnections);
+			while (conn != nullptr)
 			{
 			{
-				for (UINT32 i = 0; i < internalData->mConnections.size(); i++)
-				{
-					if (!internalData->mConnections[i]->isValid)
-					{
-						internalData->mConnections.erase(internalData->mConnections.begin() + i);
-						i--;
-					}
-				}
-
-				internalData->mHasDisconnectedCallbacks = false;
-			}
+				// Save next here in case the callback itself disconnects this connection
+				ConnectionData* next = static_cast<ConnectionData*>(conn->next);
+				
+				if (conn->func != nullptr)
+					conn->func(args...);
 
 
-			// Do not use an iterator here, as new connections might be added during iteration from
-			// the notify callback
-			UINT32 numConnections = (UINT32)internalData->mConnections.size(); // Remember current num. connections as we don't want to notify new ones
-			for (UINT32 i = 0; i < numConnections; i++)
-			{
-				if (internalData->mConnections[i]->func != nullptr)
-					internalData->mConnections[i]->func(args...);
+				conn = next;
 			}
 			}
 		}
 		}
 
 
@@ -161,16 +278,7 @@ namespace BansheeEngine
 		 */
 		 */
 		void clear()
 		void clear()
 		{
 		{
-			BS_LOCK_RECURSIVE_MUTEX(mInternalData->mMutex);
-
-			for (auto& connection : mInternalData->mConnections)
-			{
-				connection->isValid = false;
-				connection->func = nullptr;
-			}
-
-			if (mInternalData->mConnections.size() > 0)
-				mInternalData->mHasDisconnectedCallbacks = true;
+			mInternalData->clear();
 		}
 		}
 
 
 		/**
 		/**
@@ -182,43 +290,11 @@ namespace BansheeEngine
 		{
 		{
 			BS_LOCK_RECURSIVE_MUTEX(mInternalData->mMutex);
 			BS_LOCK_RECURSIVE_MUTEX(mInternalData->mMutex);
 
 
-			return mInternalData->mConnections.size() == 0;
+			return mInternalData->mConnections == nullptr;
 		}
 		}
 
 
 	private:
 	private:
-		std::shared_ptr<InternalData> mInternalData;
-
-		/**
-		 * @brief	Callback triggered by event handles when they want to disconnect from
-		 *			an event.
-		 */
-		static void disconnectCallback(const std::shared_ptr<BaseConnectionData>& connection, void* event)
-		{
-			TEvent<RetType, Args...>* castEvent = reinterpret_cast<TEvent<RetType, Args...>*>(event);
-
-			castEvent->disconnect(connection);
-		}
-
-		/**
-		 * @brief	Internal method that disconnects the callback described by the provided connection data.
-		 */
-		void disconnect(const std::shared_ptr<BaseConnectionData>& connData)
-		{
-			BS_LOCK_RECURSIVE_MUTEX(mInternalData->mMutex);
-
-			std::shared_ptr<ConnectionData> myConnData = std::static_pointer_cast<ConnectionData>(connData);
-
-			for (auto& iter = mInternalData->mConnections.begin(); iter != mInternalData->mConnections.end(); ++iter)
-			{
-				if ((*iter) == myConnData)
-				{
-					myConnData->isValid = false;
-					myConnData->func = nullptr;
-					mInternalData->mHasDisconnectedCallbacks = true;
-					return;
-				}
-			}
-		}
+		SPtr<EventInternalData> mInternalData;
 	};
 	};
 
 
 	/************************************************************************/
 	/************************************************************************/

+ 2 - 0
TODO.txt

@@ -58,6 +58,8 @@ Code quality improvements:
 ----------------------------------------------------------------------
 ----------------------------------------------------------------------
 Polish stage 1
 Polish stage 1
 
 
+Handle seems to lag behind the selected mesh
+
 Track down the large performance spike
 Track down the large performance spike
  - Seems to be caused by something on core thread
  - Seems to be caused by something on core thread
 ProjectLibrary seems to import some files on every start-up
 ProjectLibrary seems to import some files on every start-up