From 14d2085e35bbc3e5c73c018e5c70e8093003820f Mon Sep 17 00:00:00 2001 From: Tycho Bickerstaff Date: Sat, 21 Dec 2013 14:43:32 +0000 Subject: basic threadsafe queue interface --- src/OSSupport/Queue.h | 30 ++++++++++++++++++++++++++++++ src/OSSupport/Queue.inc | 4 ++++ 2 files changed, 34 insertions(+) create mode 100644 src/OSSupport/Queue.h create mode 100644 src/OSSupport/Queue.inc diff --git a/src/OSSupport/Queue.h b/src/OSSupport/Queue.h new file mode 100644 index 000000000..838a101e0 --- /dev/null +++ b/src/OSSupport/Queue.h @@ -0,0 +1,30 @@ +#pragma once + +template +class cDeleter +{ + public: + static void Delete(T) {}; +} + +template +class cQueue +{ +public: + cQueue(int warnsize); + cQueue(cQueue& queue); + ~cQueue(); + + void EnqueueItem(T item); + bool TryDequeueItem(T& item); + T DequeueItem(); + void BlockTillEmpty(cEvent CancelationEvent); + void Clear(); + int Size(); + +private: + int warnsize; +} + +//template classes must be implemented in the header +#include "Queue.inc" diff --git a/src/OSSupport/Queue.inc b/src/OSSupport/Queue.inc new file mode 100644 index 000000000..f172e9794 --- /dev/null +++ b/src/OSSupport/Queue.inc @@ -0,0 +1,4 @@ + +#include "Globals.h" // NOTE: MSVC stupidness requires this to be the same across all modules + + -- cgit v1.2.3 From 1f938b721b762a608efb8faee1913904c2cb1a77 Mon Sep 17 00:00:00 2001 From: Tycho Bickerstaff Date: Sun, 22 Dec 2013 16:26:45 +0000 Subject: added mergetool files to gitignore --- .gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index 977122047..b15b6d502 100644 --- a/.gitignore +++ b/.gitignore @@ -19,6 +19,7 @@ cloc.xsl ## emacs *.*~ *~ +*.orig # world inside source ChunkWorx.ini -- cgit v1.2.3 From d0cd9a2b36749b17f6c06998f5a766e498996b8a Mon Sep 17 00:00:00 2001 From: Tycho Bickerstaff Date: Sun, 22 Dec 2013 22:52:21 +0000 Subject: added link dependency between WorldStorage and OSSupport --- src/WorldStorage/CMakeLists.txt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/WorldStorage/CMakeLists.txt b/src/WorldStorage/CMakeLists.txt index d431bdf6a..2c83c4662 100644 --- a/src/WorldStorage/CMakeLists.txt +++ b/src/WorldStorage/CMakeLists.txt @@ -9,3 +9,5 @@ file(GLOB SOURCE ) add_library(WorldStorage ${SOURCE}) + +target_link_libraries(WorldStorage OSSupport) -- cgit v1.2.3 From f3736b1eb7bf698518cdb853ee29ee96b9c24a52 Mon Sep 17 00:00:00 2001 From: Tycho Bickerstaff Date: Tue, 31 Dec 2013 15:48:57 +0000 Subject: refactored chunk Queue to seperate class --- src/ChunkDef.h | 1 - src/OSSupport/Event.h | 6 +- src/OSSupport/IsThread.h | 1 - src/OSSupport/Queue.h | 110 +++++++++++++++++++++++++----- src/OSSupport/Queue.inc | 4 -- src/WorldStorage/WorldStorage.cpp | 136 ++++++++++++-------------------------- src/WorldStorage/WorldStorage.h | 28 ++++++-- 7 files changed, 162 insertions(+), 124 deletions(-) delete mode 100644 src/OSSupport/Queue.inc diff --git a/src/ChunkDef.h b/src/ChunkDef.h index bf41aa625..c68ae5106 100644 --- a/src/ChunkDef.h +++ b/src/ChunkDef.h @@ -14,7 +14,6 @@ - /** This is really only a placeholder to be used in places where we need to "make up" a chunk's Y coord. It will help us when the new chunk format comes out and we need to patch everything up for compatibility. */ diff --git a/src/OSSupport/Event.h b/src/OSSupport/Event.h index 71f418c0c..31f32f8c6 100644 --- a/src/OSSupport/Event.h +++ b/src/OSSupport/Event.h @@ -20,10 +20,10 @@ class cEvent { public: cEvent(void); - ~cEvent(); + virtual ~cEvent(); - void Wait(void); - void Set (void); + virtual void Wait(void); + virtual void Set (void); private: diff --git a/src/OSSupport/IsThread.h b/src/OSSupport/IsThread.h index b8784ea33..af4367636 100644 --- a/src/OSSupport/IsThread.h +++ b/src/OSSupport/IsThread.h @@ -22,7 +22,6 @@ In the descending class' constructor call the Start() method to start the thread - class cIsThread { protected: diff --git a/src/OSSupport/Queue.h b/src/OSSupport/Queue.h index 838a101e0..eb323b067 100644 --- a/src/OSSupport/Queue.h +++ b/src/OSSupport/Queue.h @@ -1,30 +1,104 @@ + #pragma once +#include + +#include "../OSSupport/Promise.h" + +//this empty struct allows function inlining template -class cDeleter +struct cQueueFuncs { public: static void Delete(T) {}; -} + static void Combine(T&, const T) {}; +}; -template +template > class cQueue { + +typedef typename std::list ListType; +//magic typedef to persuade clang that the iterator is a type +typedef typename ListType::iterator iterator; public: - cQueue(int warnsize); - cQueue(cQueue& queue); - ~cQueue(); - - void EnqueueItem(T item); - bool TryDequeueItem(T& item); - T DequeueItem(); - void BlockTillEmpty(cEvent CancelationEvent); - void Clear(); - int Size(); - + cQueue() {} + ~cQueue() {} + + void EnqueueItem(ItemType a_item) + { + cCSLock Lock(m_CS); + m_contents.push_back(a_item); + m_evtAdded.Set(); + } + void EnqueueItemIfNotPresent(ItemType a_item) + { + cCSLock Lock(m_CS); + + for (iterator itr = m_contents.begin(); itr != m_contents.end(); ++itr) + { + if((*itr) == a_item) { + Funcs funcTable; + funcTable.Combine(*itr,a_item); + return; + } + } + m_contents.push_back(a_item); + m_evtAdded.Set(); + } + bool TryDequeueItem(ItemType& item) + { + cCSLock Lock(m_CS); + if (m_contents.size() == 0) return false; + item = m_contents.front(); + m_contents.pop_front(); + return true; + } + ItemType DequeueItem() + { + cCSLock Lock(m_CS); + while (m_contents.size() == 0) + { + cCSUnlock Unlock(m_CS); + m_evtAdded.Wait(); + } + return m_contents.pop_front(); + } + cPromise* BlockTillEmpty() { + return new cEmptyQueuePromise(this); + } + //can all be inlined when delete is a noop + void Clear() + { + cCSLock Lock(m_CS); + Funcs funcTable; + while (!m_contents.empty()) + { + funcTable.Delete(m_contents.front()); + m_contents.pop_front(); + } + } + size_t Size() + { + cCSLock Lock(m_CS); + return m_contents.size(); + } + bool Remove(ItemType item) + { + cCSLock Lock(m_CS); + m_contents.remove(item); + } + private: - int warnsize; -} + ListType m_contents; + cCriticalSection m_CS; + cEvent m_evtAdded; -//template classes must be implemented in the header -#include "Queue.inc" + class cEmptyQueuePromise : public cPromise { + public: + cEmptyQueuePromise(cQueue* a_Queue) : cPromise(), m_Queue(a_Queue) {} + virtual bool IsCompleted() {return m_Queue->Size() != 0;} + private: + cQueue* m_Queue; + }; +}; diff --git a/src/OSSupport/Queue.inc b/src/OSSupport/Queue.inc deleted file mode 100644 index f172e9794..000000000 --- a/src/OSSupport/Queue.inc +++ /dev/null @@ -1,4 +0,0 @@ - -#include "Globals.h" // NOTE: MSVC stupidness requires this to be the same across all modules - - diff --git a/src/WorldStorage/WorldStorage.cpp b/src/WorldStorage/WorldStorage.cpp index f290ec128..c3bfbd4f6 100644 --- a/src/WorldStorage/WorldStorage.cpp +++ b/src/WorldStorage/WorldStorage.cpp @@ -13,7 +13,7 @@ #include "../Generating/ChunkGenerator.h" #include "../Entities/Entity.h" #include "../BlockEntities/BlockEntity.h" - +#include "../OSSupport/Promise.h" @@ -63,8 +63,6 @@ cWorldStorage::~cWorldStorage() { delete *itr; } // for itr - m_Schemas[] - m_LoadQueue.clear(); - m_SaveQueue.clear(); } @@ -98,9 +96,7 @@ void cWorldStorage::WaitForFinish(void) LOG("Waiting for the world storage to finish saving"); { - // Cancel all loading requests: - cCSLock Lock(m_CSQueues); - m_LoadQueue.clear(); + m_LoadQueue.Clear(); } // Wait for the saving to finish: @@ -120,32 +116,36 @@ void cWorldStorage::WaitForFinish(void) void cWorldStorage::WaitForQueuesEmpty(void) { - cCSLock Lock(m_CSQueues); - while (!m_ShouldTerminate && (!m_LoadQueue.empty() || !m_SaveQueue.empty())) - { - cCSUnlock Unlock(Lock); - m_evtRemoved.Wait(); - } + + cPromise * LoadPromise = m_LoadQueue.BlockTillEmpty(); + cPromise * SavePromise = m_SaveQueue.BlockTillEmpty(); + cPromise * QueuePromise = LoadPromise->WaitFor(SavePromise); + cPromise * CancelPromise = QueuePromise->CancelOn(m_ShouldTerminate); + CancelPromise->Wait(); + delete CancelPromise; + delete QueuePromise; + delete SavePromise; + delete LoadPromise; } -int cWorldStorage::GetLoadQueueLength(void) +size_t cWorldStorage::GetLoadQueueLength(void) { cCSLock Lock(m_CSQueues); - return (int)m_LoadQueue.size(); + return m_LoadQueue.Size(); } -int cWorldStorage::GetSaveQueueLength(void) +size_t cWorldStorage::GetSaveQueueLength(void) { cCSLock Lock(m_CSQueues); - return (int)m_SaveQueue.size(); + return m_SaveQueue.Size(); } @@ -154,22 +154,7 @@ int cWorldStorage::GetSaveQueueLength(void) void cWorldStorage::QueueLoadChunk(int a_ChunkX, int a_ChunkY, int a_ChunkZ, bool a_Generate) { - // Queues the chunk for loading; if not loaded, the chunk will be generated - { - cCSLock Lock(m_CSQueues); - - // Check if already in the queue: - for (sChunkLoadQueue::iterator itr = m_LoadQueue.begin(); itr != m_LoadQueue.end(); ++itr) - { - if ((itr->m_ChunkX == a_ChunkX) && (itr->m_ChunkY == a_ChunkY) && (itr->m_ChunkZ == a_ChunkZ) && (itr->m_Generate == a_Generate)) - { - return; - } - } - m_LoadQueue.push_back(sChunkLoad(a_ChunkX, a_ChunkY, a_ChunkZ, a_Generate)); - } - - m_Event.Set(); + m_LoadQueue.EnqueueItemIfNotPresent(sChunkLoad(a_ChunkX, a_ChunkY, a_ChunkZ, a_Generate)); } @@ -178,12 +163,7 @@ void cWorldStorage::QueueLoadChunk(int a_ChunkX, int a_ChunkY, int a_ChunkZ, boo void cWorldStorage::QueueSaveChunk(int a_ChunkX, int a_ChunkY, int a_ChunkZ) { - { - cCSLock Lock(m_CSQueues); - m_SaveQueue.remove (cChunkCoords(a_ChunkX, a_ChunkY, a_ChunkZ)); // Don't add twice - m_SaveQueue.push_back(cChunkCoords(a_ChunkX, a_ChunkY, a_ChunkZ)); - } - m_Event.Set(); + m_SaveQueue.EnqueueItemIfNotPresent(cChunkCoords(a_ChunkX, a_ChunkY, a_ChunkZ)); } @@ -192,12 +172,8 @@ void cWorldStorage::QueueSaveChunk(int a_ChunkX, int a_ChunkY, int a_ChunkZ) void cWorldStorage::QueueSavedMessage(void) { - // Pushes a special coord pair into the queue, signalizing a message instead: - { - cCSLock Lock(m_CSQueues); - m_SaveQueue.push_back(cChunkCoords(0, CHUNK_Y_MESSAGE, 0)); - } - m_Event.Set(); + // Pushes a special coord pair into the queue, signalizing a message instead + m_SaveQueue.EnqueueItem(cChunkCoords(0, CHUNK_Y_MESSAGE, 0)); } @@ -206,7 +182,7 @@ void cWorldStorage::QueueSavedMessage(void) void cWorldStorage::UnqueueLoad(int a_ChunkX, int a_ChunkY, int a_ChunkZ) { - cCSLock Lock(m_CSQueues); + /*cCSLock Lock(m_CSQueues); for (sChunkLoadQueue::iterator itr = m_LoadQueue.begin(); itr != m_LoadQueue.end(); ++itr) { if ((itr->m_ChunkX != a_ChunkX) || (itr->m_ChunkY != a_ChunkY) || (itr->m_ChunkZ != a_ChunkZ)) @@ -217,7 +193,8 @@ void cWorldStorage::UnqueueLoad(int a_ChunkX, int a_ChunkY, int a_ChunkZ) Lock.Unlock(); m_evtRemoved.Set(); return; - } // for itr - m_LoadQueue[] + } // for itr - m_LoadQueue[]*/ + m_LoadQueue.Remove(sChunkLoad(a_ChunkX, a_ChunkY, a_ChunkZ,true)); } @@ -226,11 +203,7 @@ void cWorldStorage::UnqueueLoad(int a_ChunkX, int a_ChunkY, int a_ChunkZ) void cWorldStorage::UnqueueSave(const cChunkCoords & a_Chunk) { - { - cCSLock Lock(m_CSQueues); - m_SaveQueue.remove(a_Chunk); - } - m_evtRemoved.Set(); + m_SaveQueue.Remove(a_Chunk); } @@ -281,19 +254,19 @@ void cWorldStorage::Execute(void) m_Event.Wait(); // Process both queues until they are empty again: - bool HasMore; + bool Success; do { - HasMore = false; + Success = false; if (m_ShouldTerminate) { return; } - HasMore = LoadOneChunk(); - HasMore = HasMore | SaveOneChunk(); + Success = LoadOneChunk(); + Success |= SaveOneChunk(); m_evtRemoved.Set(); - } while (HasMore); + } while (Success); } } @@ -304,19 +277,7 @@ void cWorldStorage::Execute(void) bool cWorldStorage::LoadOneChunk(void) { sChunkLoad ToLoad(0, 0, 0, false); - bool HasMore; - bool ShouldLoad = false; - { - cCSLock Lock(m_CSQueues); - if (!m_LoadQueue.empty()) - { - ToLoad = m_LoadQueue.front(); - m_LoadQueue.pop_front(); - ShouldLoad = true; - } - HasMore = !m_LoadQueue.empty(); - } - + bool ShouldLoad = m_LoadQueue.TryDequeueItem(ToLoad); if (ShouldLoad && !LoadChunk(ToLoad.m_ChunkX, ToLoad.m_ChunkY, ToLoad.m_ChunkZ)) { if (ToLoad.m_Generate) @@ -330,7 +291,7 @@ bool cWorldStorage::LoadOneChunk(void) // m_World->ChunkLoadFailed(ToLoad.m_ChunkX, ToLoad.m_ChunkY, ToLoad.m_ChunkZ); } } - return HasMore; + return ShouldLoad; } @@ -339,33 +300,24 @@ bool cWorldStorage::LoadOneChunk(void) bool cWorldStorage::SaveOneChunk(void) { - cChunkCoords Save(0, 0, 0); - bool HasMore; - bool ShouldSave = false; - { - cCSLock Lock(m_CSQueues); - if (!m_SaveQueue.empty()) + cChunkCoords ToSave(0, 0, 0); + bool ShouldSave = m_SaveQueue.TryDequeueItem(ToSave); + if(ShouldSave) { + if (ToSave.m_ChunkY == CHUNK_Y_MESSAGE) { - Save = m_SaveQueue.front(); - m_SaveQueue.pop_front(); - ShouldSave = true; + LOGINFO("Saved all chunks in world %s", m_World->GetName().c_str()); + return ShouldSave; } - HasMore = !m_SaveQueue.empty(); - } - if (Save.m_ChunkY == CHUNK_Y_MESSAGE) - { - LOGINFO("Saved all chunks in world %s", m_World->GetName().c_str()); - return HasMore; - } - if (ShouldSave && m_World->IsChunkValid(Save.m_ChunkX, Save.m_ChunkZ)) - { - m_World->MarkChunkSaving(Save.m_ChunkX, Save.m_ChunkZ); - if (m_SaveSchema->SaveChunk(Save)) + if (ShouldSave && m_World->IsChunkValid(ToSave.m_ChunkX, ToSave.m_ChunkZ)) { - m_World->MarkChunkSaved(Save.m_ChunkX, Save.m_ChunkZ); + m_World->MarkChunkSaving(ToSave.m_ChunkX, ToSave.m_ChunkZ); + if (m_SaveSchema->SaveChunk(ToSave)) + { + m_World->MarkChunkSaved(ToSave.m_ChunkX, ToSave.m_ChunkZ); + } } } - return HasMore; + return ShouldSave; } diff --git a/src/WorldStorage/WorldStorage.h b/src/WorldStorage/WorldStorage.h index 007d37571..c3eb96ce8 100644 --- a/src/WorldStorage/WorldStorage.h +++ b/src/WorldStorage/WorldStorage.h @@ -16,6 +16,7 @@ #include "../ChunkDef.h" #include "../OSSupport/IsThread.h" +#include "../OSSupport/Queue.h" @@ -24,6 +25,8 @@ // fwd: class cWorld; +typedef cQueue cChunkCoordsQueue; + @@ -78,8 +81,8 @@ public: void WaitForFinish(void); void WaitForQueuesEmpty(void); - int GetLoadQueueLength(void); - int GetSaveQueueLength(void); + size_t GetLoadQueueLength(void); + size_t GetSaveQueueLength(void); protected: @@ -91,9 +94,24 @@ protected: bool m_Generate; // If true, the chunk will be generated if it cannot be loaded sChunkLoad(int a_ChunkX, int a_ChunkY, int a_ChunkZ, bool a_Generate) : m_ChunkX(a_ChunkX), m_ChunkY(a_ChunkY), m_ChunkZ(a_ChunkZ), m_Generate(a_Generate) {} + + bool operator==(const sChunkLoad other) const + { + return this->m_ChunkX == other.m_ChunkX && + this->m_ChunkY == other.m_ChunkY && + this->m_ChunkZ == other.m_ChunkZ; + } } ; - - typedef std::list sChunkLoadQueue; + + struct FuncTable { + static void Delete(sChunkLoad) {}; + static void Combine(sChunkLoad& a_orig, const sChunkLoad a_new) + { + a_orig.m_Generate |= a_new.m_Generate; + }; + }; + + typedef cQueue sChunkLoadQueue; cWorld * m_World; AString m_StorageSchemaName; @@ -101,7 +119,7 @@ protected: // Both queues are locked by the same CS cCriticalSection m_CSQueues; sChunkLoadQueue m_LoadQueue; - cChunkCoordsList m_SaveQueue; + cChunkCoordsQueue m_SaveQueue; cEvent m_Event; // Set when there's any addition to the queues cEvent m_evtRemoved; // Set when an item has been removed from the queue, either by the worker thread or the Unqueue methods -- cgit v1.2.3 From 512d1b9ebebaf205756dde352c4e714dd632ca2d Mon Sep 17 00:00:00 2001 From: Tycho Bickerstaff Date: Tue, 31 Dec 2013 15:58:21 +0000 Subject: clean up code for patching --- src/ChunkDef.h | 1 + src/OSSupport/Event.h | 6 +++--- src/OSSupport/IsThread.h | 1 + 3 files changed, 5 insertions(+), 3 deletions(-) diff --git a/src/ChunkDef.h b/src/ChunkDef.h index 8919bcbb8..7d727a4d4 100644 --- a/src/ChunkDef.h +++ b/src/ChunkDef.h @@ -14,6 +14,7 @@ + /** This is really only a placeholder to be used in places where we need to "make up" a chunk's Y coord. It will help us when the new chunk format comes out and we need to patch everything up for compatibility. */ diff --git a/src/OSSupport/Event.h b/src/OSSupport/Event.h index 31f32f8c6..71f418c0c 100644 --- a/src/OSSupport/Event.h +++ b/src/OSSupport/Event.h @@ -20,10 +20,10 @@ class cEvent { public: cEvent(void); - virtual ~cEvent(); + ~cEvent(); - virtual void Wait(void); - virtual void Set (void); + void Wait(void); + void Set (void); private: diff --git a/src/OSSupport/IsThread.h b/src/OSSupport/IsThread.h index af4367636..b8784ea33 100644 --- a/src/OSSupport/IsThread.h +++ b/src/OSSupport/IsThread.h @@ -22,6 +22,7 @@ In the descending class' constructor call the Start() method to start the thread + class cIsThread { protected: -- cgit v1.2.3 From 098ed91a48df9fef58590b22fe34225ed1f44cfa Mon Sep 17 00:00:00 2001 From: Tycho Bickerstaff Date: Tue, 31 Dec 2013 16:13:13 +0000 Subject: fogot to add promise classes --- src/OSSupport/Promise.cpp | 54 +++++++++++++++++++++++++++++++++++++++++++++++ src/OSSupport/Promise.h | 38 +++++++++++++++++++++++++++++++++ 2 files changed, 92 insertions(+) create mode 100644 src/OSSupport/Promise.cpp create mode 100644 src/OSSupport/Promise.h diff --git a/src/OSSupport/Promise.cpp b/src/OSSupport/Promise.cpp new file mode 100644 index 000000000..b31869334 --- /dev/null +++ b/src/OSSupport/Promise.cpp @@ -0,0 +1,54 @@ + +#include "Globals.h" + +#include "Promise.h" + +cPromise * cPromise::WaitFor(cPromise * a_Promise) +{ + return new cCombinedPromise(this, a_Promise); +} + +cPromise * cPromise::CancelOn(volatile bool& cancelation) +{ + return new cCancelablePromise(this, cancelation); +} + +void cPromise::Wait() +{ + while(!IsCompleted()){}; //busywait best we can do until waitany +} + + +cCombinedPromise::cCombinedPromise(cPromise* a_left, cPromise* a_right) : + cPromise(), + m_left(a_left), + m_right(a_right) +{ +} + +cCombinedPromise::~cCombinedPromise() +{ +} + +bool cCombinedPromise::IsCompleted() +{ + return m_left->IsCompleted() || m_right->IsCompleted(); +} + +cCancelablePromise::cCancelablePromise(cPromise* a_wrapped, volatile bool& a_cancel) : + cPromise(), + m_cancel(a_cancel), + m_wrapped(a_wrapped) +{ +} + +cCancelablePromise::~cCancelablePromise () +{ +} + +bool cCancelablePromise::IsCompleted() +{ + return m_cancel || m_wrapped->IsCompleted(); +} + + diff --git a/src/OSSupport/Promise.h b/src/OSSupport/Promise.h new file mode 100644 index 000000000..83d04860b --- /dev/null +++ b/src/OSSupport/Promise.h @@ -0,0 +1,38 @@ +#pragma once + +class cCombinedPromise; + + +class cPromise { + public: + cPromise() {} + virtual ~cPromise () {} + cPromise * WaitFor(cPromise * a_Promise); + cPromise * CancelOn(volatile bool& cancelationtoken); + void Wait(); + virtual bool IsCompleted() = 0; + //TODO:Expose Events for waiting on +}; + +class cCombinedPromise : public cPromise { +public: + cCombinedPromise(cPromise*, cPromise*); + ~cCombinedPromise(); + virtual bool IsCompleted(); +private: + cPromise* m_left; + cPromise* m_right; +}; + +class cCancelablePromise : public cPromise { +public: + cCancelablePromise(cPromise*, volatile bool&); + ~cCancelablePromise(); + virtual bool IsCompleted(); +private: + volatile bool& m_cancel; + cPromise* m_wrapped; +}; + + + -- cgit v1.2.3 From 042b72bc172e7eb4e9ef7668ae28be6e7a3b4036 Mon Sep 17 00:00:00 2001 From: Tycho Bickerstaff Date: Thu, 2 Jan 2014 12:32:55 +0000 Subject: rewrote queue not to use promises for waits --- src/OSSupport/Promise.cpp | 54 --------------------------------------- src/OSSupport/Promise.h | 38 --------------------------- src/OSSupport/Queue.h | 24 ++++++++--------- src/World.cpp | 5 +++- src/WorldStorage/WorldStorage.cpp | 21 +++++---------- src/WorldStorage/WorldStorage.h | 3 ++- 6 files changed, 24 insertions(+), 121 deletions(-) delete mode 100644 src/OSSupport/Promise.cpp delete mode 100644 src/OSSupport/Promise.h diff --git a/src/OSSupport/Promise.cpp b/src/OSSupport/Promise.cpp deleted file mode 100644 index b31869334..000000000 --- a/src/OSSupport/Promise.cpp +++ /dev/null @@ -1,54 +0,0 @@ - -#include "Globals.h" - -#include "Promise.h" - -cPromise * cPromise::WaitFor(cPromise * a_Promise) -{ - return new cCombinedPromise(this, a_Promise); -} - -cPromise * cPromise::CancelOn(volatile bool& cancelation) -{ - return new cCancelablePromise(this, cancelation); -} - -void cPromise::Wait() -{ - while(!IsCompleted()){}; //busywait best we can do until waitany -} - - -cCombinedPromise::cCombinedPromise(cPromise* a_left, cPromise* a_right) : - cPromise(), - m_left(a_left), - m_right(a_right) -{ -} - -cCombinedPromise::~cCombinedPromise() -{ -} - -bool cCombinedPromise::IsCompleted() -{ - return m_left->IsCompleted() || m_right->IsCompleted(); -} - -cCancelablePromise::cCancelablePromise(cPromise* a_wrapped, volatile bool& a_cancel) : - cPromise(), - m_cancel(a_cancel), - m_wrapped(a_wrapped) -{ -} - -cCancelablePromise::~cCancelablePromise () -{ -} - -bool cCancelablePromise::IsCompleted() -{ - return m_cancel || m_wrapped->IsCompleted(); -} - - diff --git a/src/OSSupport/Promise.h b/src/OSSupport/Promise.h deleted file mode 100644 index 83d04860b..000000000 --- a/src/OSSupport/Promise.h +++ /dev/null @@ -1,38 +0,0 @@ -#pragma once - -class cCombinedPromise; - - -class cPromise { - public: - cPromise() {} - virtual ~cPromise () {} - cPromise * WaitFor(cPromise * a_Promise); - cPromise * CancelOn(volatile bool& cancelationtoken); - void Wait(); - virtual bool IsCompleted() = 0; - //TODO:Expose Events for waiting on -}; - -class cCombinedPromise : public cPromise { -public: - cCombinedPromise(cPromise*, cPromise*); - ~cCombinedPromise(); - virtual bool IsCompleted(); -private: - cPromise* m_left; - cPromise* m_right; -}; - -class cCancelablePromise : public cPromise { -public: - cCancelablePromise(cPromise*, volatile bool&); - ~cCancelablePromise(); - virtual bool IsCompleted(); -private: - volatile bool& m_cancel; - cPromise* m_wrapped; -}; - - - diff --git a/src/OSSupport/Queue.h b/src/OSSupport/Queue.h index eb323b067..153e201c1 100644 --- a/src/OSSupport/Queue.h +++ b/src/OSSupport/Queue.h @@ -3,8 +3,6 @@ #include -#include "../OSSupport/Promise.h" - //this empty struct allows function inlining template struct cQueueFuncs @@ -52,6 +50,7 @@ public: if (m_contents.size() == 0) return false; item = m_contents.front(); m_contents.pop_front(); + m_evtRemoved.Set(); return true; } ItemType DequeueItem() @@ -62,10 +61,15 @@ public: cCSUnlock Unlock(m_CS); m_evtAdded.Wait(); } - return m_contents.pop_front(); + ItemType item = m_contents.front(); + m_contents.pop_front(); + m_evtRemoved.Set(); + return item; } - cPromise* BlockTillEmpty() { - return new cEmptyQueuePromise(this); + void BlockTillEmpty() { + //There is a very slight race condition here if the load completes between the check + //and the wait. + while(!(Size() == 0)){m_evtRemoved.Wait();} } //can all be inlined when delete is a noop void Clear() @@ -87,18 +91,12 @@ public: { cCSLock Lock(m_CS); m_contents.remove(item); + m_evtRemoved.Set(); } private: ListType m_contents; cCriticalSection m_CS; cEvent m_evtAdded; - - class cEmptyQueuePromise : public cPromise { - public: - cEmptyQueuePromise(cQueue* a_Queue) : cPromise(), m_Queue(a_Queue) {} - virtual bool IsCompleted() {return m_Queue->Size() != 0;} - private: - cQueue* m_Queue; - }; + cEvent m_evtRemoved; }; diff --git a/src/World.cpp b/src/World.cpp index cc543d460..39300d419 100644 --- a/src/World.cpp +++ b/src/World.cpp @@ -367,10 +367,13 @@ void cWorld::InitializeSpawn(void) cWorldLoadProgress Progress(this); // Wait for the loader to finish loading - m_Storage.WaitForQueuesEmpty(); + m_Storage.WaitForLoadQueueEmpty(); // Wait for the generator to finish generating m_Generator.WaitForQueueEmpty(); + + // Wait for the loader to finish saving + m_Storage.WaitForSaveQueueEmpty(); Progress.Stop(); } diff --git a/src/WorldStorage/WorldStorage.cpp b/src/WorldStorage/WorldStorage.cpp index c3bfbd4f6..9ad995c82 100644 --- a/src/WorldStorage/WorldStorage.cpp +++ b/src/WorldStorage/WorldStorage.cpp @@ -13,7 +13,6 @@ #include "../Generating/ChunkGenerator.h" #include "../Entities/Entity.h" #include "../BlockEntities/BlockEntity.h" -#include "../OSSupport/Promise.h" @@ -100,7 +99,7 @@ void cWorldStorage::WaitForFinish(void) } // Wait for the saving to finish: - WaitForQueuesEmpty(); + WaitForSaveQueueEmpty(); // Wait for the thread to finish: m_ShouldTerminate = true; @@ -114,21 +113,15 @@ void cWorldStorage::WaitForFinish(void) -void cWorldStorage::WaitForQueuesEmpty(void) +void cWorldStorage::WaitForLoadQueueEmpty(void) { - - cPromise * LoadPromise = m_LoadQueue.BlockTillEmpty(); - cPromise * SavePromise = m_SaveQueue.BlockTillEmpty(); - cPromise * QueuePromise = LoadPromise->WaitFor(SavePromise); - cPromise * CancelPromise = QueuePromise->CancelOn(m_ShouldTerminate); - CancelPromise->Wait(); - delete CancelPromise; - delete QueuePromise; - delete SavePromise; - delete LoadPromise; + m_LoadQueue.BlockTillEmpty(); } - +void cWorldStorage::WaitForSaveQueueEmpty(void) +{ + m_SaveQueue.BlockTillEmpty(); +} diff --git a/src/WorldStorage/WorldStorage.h b/src/WorldStorage/WorldStorage.h index c3eb96ce8..98eb5fce7 100644 --- a/src/WorldStorage/WorldStorage.h +++ b/src/WorldStorage/WorldStorage.h @@ -79,7 +79,8 @@ public: bool Start(cWorld * a_World, const AString & a_StorageSchemaName); // Hide the cIsThread's Start() method, we need to provide args void Stop(void); // Hide the cIsThread's Stop() method, we need to signal the event void WaitForFinish(void); - void WaitForQueuesEmpty(void); + void WaitForLoadQueueEmpty(void); + void WaitForSaveQueueEmpty(void); size_t GetLoadQueueLength(void); size_t GetSaveQueueLength(void); -- cgit v1.2.3 From bbdb34252e9c1023405b58585fd5999cc8f39b45 Mon Sep 17 00:00:00 2001 From: Tycho Bickerstaff Date: Thu, 2 Jan 2014 17:37:34 +0000 Subject: fixed a few remaining issues with worldstorage --- src/WorldStorage/WorldStorage.cpp | 21 +++------------------ src/WorldStorage/WorldStorage.h | 9 +++------ 2 files changed, 6 insertions(+), 24 deletions(-) diff --git a/src/WorldStorage/WorldStorage.cpp b/src/WorldStorage/WorldStorage.cpp index 9ad995c82..5f4c112d5 100644 --- a/src/WorldStorage/WorldStorage.cpp +++ b/src/WorldStorage/WorldStorage.cpp @@ -103,8 +103,7 @@ void cWorldStorage::WaitForFinish(void) // Wait for the thread to finish: m_ShouldTerminate = true; - m_Event.Set(); - m_evtRemoved.Set(); // Wake up anybody waiting in the WaitForQueuesEmpty() method + m_Event.Set(); // Wake up the thread if waiting super::Wait(); LOG("World storage thread finished"); } @@ -127,7 +126,6 @@ void cWorldStorage::WaitForSaveQueueEmpty(void) size_t cWorldStorage::GetLoadQueueLength(void) { - cCSLock Lock(m_CSQueues); return m_LoadQueue.Size(); } @@ -137,7 +135,6 @@ size_t cWorldStorage::GetLoadQueueLength(void) size_t cWorldStorage::GetSaveQueueLength(void) { - cCSLock Lock(m_CSQueues); return m_SaveQueue.Size(); } @@ -147,6 +144,7 @@ size_t cWorldStorage::GetSaveQueueLength(void) void cWorldStorage::QueueLoadChunk(int a_ChunkX, int a_ChunkY, int a_ChunkZ, bool a_Generate) { + m_Event.Set(); m_LoadQueue.EnqueueItemIfNotPresent(sChunkLoad(a_ChunkX, a_ChunkY, a_ChunkZ, a_Generate)); } @@ -156,6 +154,7 @@ void cWorldStorage::QueueLoadChunk(int a_ChunkX, int a_ChunkY, int a_ChunkZ, boo void cWorldStorage::QueueSaveChunk(int a_ChunkX, int a_ChunkY, int a_ChunkZ) { + m_Event.Set(); m_SaveQueue.EnqueueItemIfNotPresent(cChunkCoords(a_ChunkX, a_ChunkY, a_ChunkZ)); } @@ -175,18 +174,6 @@ void cWorldStorage::QueueSavedMessage(void) void cWorldStorage::UnqueueLoad(int a_ChunkX, int a_ChunkY, int a_ChunkZ) { - /*cCSLock Lock(m_CSQueues); - for (sChunkLoadQueue::iterator itr = m_LoadQueue.begin(); itr != m_LoadQueue.end(); ++itr) - { - if ((itr->m_ChunkX != a_ChunkX) || (itr->m_ChunkY != a_ChunkY) || (itr->m_ChunkZ != a_ChunkZ)) - { - continue; - } - m_LoadQueue.erase(itr); - Lock.Unlock(); - m_evtRemoved.Set(); - return; - } // for itr - m_LoadQueue[]*/ m_LoadQueue.Remove(sChunkLoad(a_ChunkX, a_ChunkY, a_ChunkZ,true)); } @@ -245,7 +232,6 @@ void cWorldStorage::Execute(void) while (!m_ShouldTerminate) { m_Event.Wait(); - // Process both queues until they are empty again: bool Success; do @@ -258,7 +244,6 @@ void cWorldStorage::Execute(void) Success = LoadOneChunk(); Success |= SaveOneChunk(); - m_evtRemoved.Set(); } while (Success); } } diff --git a/src/WorldStorage/WorldStorage.h b/src/WorldStorage/WorldStorage.h index 98eb5fce7..06cae1717 100644 --- a/src/WorldStorage/WorldStorage.h +++ b/src/WorldStorage/WorldStorage.h @@ -116,15 +116,10 @@ protected: cWorld * m_World; AString m_StorageSchemaName; - - // Both queues are locked by the same CS - cCriticalSection m_CSQueues; + sChunkLoadQueue m_LoadQueue; cChunkCoordsQueue m_SaveQueue; - cEvent m_Event; // Set when there's any addition to the queues - cEvent m_evtRemoved; // Set when an item has been removed from the queue, either by the worker thread or the Unqueue methods - /// All the storage schemas (all used for loading) cWSSchemaList m_Schemas; @@ -135,6 +130,8 @@ protected: virtual void Execute(void) override; + cEvent m_Event; // Set when there's any addition to the queues + /// Loads one chunk from the queue (if any queued); returns true if there are more chunks in the load queue bool LoadOneChunk(void); -- cgit v1.2.3 From d522619ce2ac05831febedea675d121445dcbdbe Mon Sep 17 00:00:00 2001 From: Tycho Bickerstaff Date: Thu, 2 Jan 2014 17:43:57 +0000 Subject: added documentation --- src/OSSupport/Queue.h | 52 ++++++++++++++++++++++++++++++++++++++++++++------- 1 file changed, 45 insertions(+), 7 deletions(-) diff --git a/src/OSSupport/Queue.h b/src/OSSupport/Queue.h index 153e201c1..1f0c19f40 100644 --- a/src/OSSupport/Queue.h +++ b/src/OSSupport/Queue.h @@ -1,34 +1,55 @@ +// Queue.h + +// Implements the cQueue class representing a thread safe queue + #pragma once #include -//this empty struct allows function inlining +/* +Usage: +To use the callback functions Delete and Combine create a class with the two +methods and pass it as a second template parameter to cQueue. The class does +not need to inherit cQueueFuncs but you do so to document the classes purpose. +The second template parmeter is optional if not overriding the callback +functions +*/ + +// this empty struct allows for the callback functions to be inlined template struct cQueueFuncs { public: + // Called when an Item is deleted form the queue without being returned static void Delete(T) {}; - static void Combine(T&, const T) {}; + // Called when an Item is inserted with EnqueueItemIfNotPresent and + // there is another equal value already inserted + static void Combine(T& a_existing, const T a_new) {}; }; template > class cQueue { - +// internal typedef for a List of Items typedef typename std::list ListType; -//magic typedef to persuade clang that the iterator is a type +// magic typedef to persuade clang that the iterator is a type typedef typename ListType::iterator iterator; public: cQueue() {} ~cQueue() {} + // Enqueues an item to the queue, may block if other threads are accessing + // the queue. void EnqueueItem(ItemType a_item) { cCSLock Lock(m_CS); m_contents.push_back(a_item); m_evtAdded.Set(); } + + // Enqueues an item to the queue if not already present as determined with + // operator ==. Will block other threads from accessing the queue. void EnqueueItemIfNotPresent(ItemType a_item) { cCSLock Lock(m_CS); @@ -44,6 +65,11 @@ public: m_contents.push_back(a_item); m_evtAdded.Set(); } + + // Dequeues an Item from the queue if any are present, provides no + // guarantees about success if the list is empty but an item is enqueued at + // the same time. Returns true if successful. Value of item is undefined if + // Dequeuing was unsuccessful. bool TryDequeueItem(ItemType& item) { cCSLock Lock(m_CS); @@ -53,6 +79,8 @@ public: m_evtRemoved.Set(); return true; } + + // Dequeues an Item from the Queue, blocking until an Item is Available. ItemType DequeueItem() { cCSLock Lock(m_CS); @@ -66,12 +94,17 @@ public: m_evtRemoved.Set(); return item; } + + // Blocks Until the queue is Empty, Has a slight race condition which may + // cause it to miss the queue being empty. void BlockTillEmpty() { - //There is a very slight race condition here if the load completes between the check - //and the wait. + // There is a very slight race condition here if the load completes between the check + // and the wait. while(!(Size() == 0)){m_evtRemoved.Wait();} } - //can all be inlined when delete is a noop + + // Removes all Items from the Queue, calling Delete on each of them. + // can all be inlined when delete is a noop void Clear() { cCSLock Lock(m_CS); @@ -82,11 +115,16 @@ public: m_contents.pop_front(); } } + + // Returns the Size at time of being called + // Do not use to detirmine weather to call DequeueItem, use TryDequeue instead size_t Size() { cCSLock Lock(m_CS); return m_contents.size(); } + + // Removes an Item from the queue bool Remove(ItemType item) { cCSLock Lock(m_CS); -- cgit v1.2.3 From 6f3c5b806eb59e2610f8bac9ad3fff24994609c9 Mon Sep 17 00:00:00 2001 From: Tycho Bickerstaff Date: Fri, 3 Jan 2014 11:22:01 +0000 Subject: implement xsofts recommendations --- src/OSSupport/Queue.h | 24 +++++++++++++++--------- 1 file changed, 15 insertions(+), 9 deletions(-) diff --git a/src/OSSupport/Queue.h b/src/OSSupport/Queue.h index 1f0c19f40..65f9bd258 100644 --- a/src/OSSupport/Queue.h +++ b/src/OSSupport/Queue.h @@ -5,15 +5,18 @@ #pragma once -#include - /* +Items can be added multiple times to a queue, there are two functions for +adding, EnqueueItem() and EnqueueItemIfNotPresent(). The first one always +enqueues the specified item, the second one checks if the item is already +present and only queues it if it isn't. + Usage: -To use the callback functions Delete and Combine create a class with the two -methods and pass it as a second template parameter to cQueue. The class does -not need to inherit cQueueFuncs but you do so to document the classes purpose. -The second template parmeter is optional if not overriding the callback -functions +To create a queue of type T, instantiate a cQueue object. You can also +modify the behavior of the queue when deleting items and when adding items +that are already in the queue by providing a second parameter, a class that +implements the functions Delete() and Combine(). An example is given in +cQueueFuncs and is used as the default behavior. */ // this empty struct allows for the callback functions to be inlined @@ -25,7 +28,7 @@ struct cQueueFuncs static void Delete(T) {}; // Called when an Item is inserted with EnqueueItemIfNotPresent and // there is another equal value already inserted - static void Combine(T& a_existing, const T a_new) {}; + static void Combine(T& a_existing, const T& a_new) {}; }; template > @@ -73,7 +76,10 @@ public: bool TryDequeueItem(ItemType& item) { cCSLock Lock(m_CS); - if (m_contents.size() == 0) return false; + if (m_contents.size() == 0) + { + return false; + } item = m_contents.front(); m_contents.pop_front(); m_evtRemoved.Set(); -- cgit v1.2.3 From 0e8bb3bf415e4c0fe6c8bd0aa06dc2d9af2823c4 Mon Sep 17 00:00:00 2001 From: Tycho Date: Fri, 3 Jan 2014 08:34:41 -0800 Subject: fixed failure to return a value from Remove --- src/OSSupport/Queue.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/OSSupport/Queue.h b/src/OSSupport/Queue.h index 65f9bd258..bf6d7e451 100644 --- a/src/OSSupport/Queue.h +++ b/src/OSSupport/Queue.h @@ -134,8 +134,8 @@ public: bool Remove(ItemType item) { cCSLock Lock(m_CS); - m_contents.remove(item); m_evtRemoved.Set(); + return m_contents.remove(item); } private: -- cgit v1.2.3 From 14ec68d8d309d3fdf8e0af47196b1cf8609d017d Mon Sep 17 00:00:00 2001 From: Tycho Date: Fri, 3 Jan 2014 08:49:14 -0800 Subject: actual fix --- src/OSSupport/Queue.h | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/OSSupport/Queue.h b/src/OSSupport/Queue.h index bf6d7e451..fc942b3e1 100644 --- a/src/OSSupport/Queue.h +++ b/src/OSSupport/Queue.h @@ -134,8 +134,15 @@ public: bool Remove(ItemType item) { cCSLock Lock(m_CS); - m_evtRemoved.Set(); - return m_contents.remove(item); + for (iterator itr = m_contents.begin(); itr != m_contents.end(); ++itr) + { + if((*itr) == a_item) { + m_contents.erase(itr); + m_evtRemoved.Set(); + return true; + } + } + return false; } private: -- cgit v1.2.3 From 13bbb3d99dee8041d47279cae3484c3083491f18 Mon Sep 17 00:00:00 2001 From: Tycho Date: Fri, 3 Jan 2014 08:56:20 -0800 Subject: derp --- lib/tolua++/Makefile | 3 +++ src/OSSupport/Queue.h | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/lib/tolua++/Makefile b/lib/tolua++/Makefile index c15b4fc80..3bbda9b15 100644 --- a/lib/tolua++/Makefile +++ b/lib/tolua++/Makefile @@ -35,6 +35,9 @@ CMAKE_COMMAND = /usr/bin/cmake # The command to remove a file. RM = /usr/bin/cmake -E remove -f +# Escaping for special characters. +EQUALS = = + # The top-level source directory on which CMake was run. CMAKE_SOURCE_DIR = /home/tycho/MCServer diff --git a/src/OSSupport/Queue.h b/src/OSSupport/Queue.h index fc942b3e1..ab42c871e 100644 --- a/src/OSSupport/Queue.h +++ b/src/OSSupport/Queue.h @@ -131,7 +131,7 @@ public: } // Removes an Item from the queue - bool Remove(ItemType item) + bool Remove(ItemType a_item) { cCSLock Lock(m_CS); for (iterator itr = m_contents.begin(); itr != m_contents.end(); ++itr) -- cgit v1.2.3 From d26c0e3815a8e3c38447eeb65b052a5d7c43da73 Mon Sep 17 00:00:00 2001 From: Tycho Date: Fri, 3 Jan 2014 09:42:35 -0800 Subject: Fixed Documentation --- src/OSSupport/Queue.h | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/OSSupport/Queue.h b/src/OSSupport/Queue.h index ab42c871e..cde26e415 100644 --- a/src/OSSupport/Queue.h +++ b/src/OSSupport/Queue.h @@ -69,10 +69,8 @@ public: m_evtAdded.Set(); } - // Dequeues an Item from the queue if any are present, provides no - // guarantees about success if the list is empty but an item is enqueued at - // the same time. Returns true if successful. Value of item is undefined if - // Dequeuing was unsuccessful. + // Dequeues an Item from the queue if any are present. Returns true if + // successful. Value of item is undefined if Dequeuing was unsuccessful. bool TryDequeueItem(ItemType& item) { cCSLock Lock(m_CS); -- cgit v1.2.3