From dd168b0e8b0b814a9e82d7941dbd0137fe09beb9 Mon Sep 17 00:00:00 2001 From: tycho Date: Wed, 16 Sep 2015 17:04:05 +0100 Subject: Removed a significant performance issue. Iterating through the list of clients in chunks was taking up a significant amount of time with larger numbers of clients due to processor stalls. Changing the data structure to a vector fixed the issue. --- src/Chunk.cpp | 140 ++++++++++++++++++++++++++-------------------------------- src/Chunk.h | 13 ++++-- 2 files changed, 71 insertions(+), 82 deletions(-) diff --git a/src/Chunk.cpp b/src/Chunk.cpp index ca7173262..6aee2eed9 100644 --- a/src/Chunk.cpp +++ b/src/Chunk.cpp @@ -199,9 +199,9 @@ void cChunk::MarkRegenerating(void) SetPresence(cpQueued); // Tell all clients attached to this chunk that they want this chunk: - for (cClientHandleList::iterator itr = m_LoadedByClient.begin(); itr != m_LoadedByClient.end(); ++itr) + for (auto ClientHandle : m_LoadedByClient) { - (*itr)->AddWantedChunk(m_PosX, m_PosZ); + ClientHandle->AddWantedChunk(m_PosX, m_PosZ); } // for itr - m_LoadedByClient[] } @@ -474,25 +474,25 @@ void cChunk::Stay(bool a_Stay) void cChunk::CollectMobCensus(cMobCensus & toFill) { toFill.CollectSpawnableChunk(*this); - std::list playerPositions; - cPlayer * currentPlayer; - for (auto itr = m_LoadedByClient.begin(), end = m_LoadedByClient.end(); itr != end; ++itr) + std::vector PlayerPositions; + PlayerPositions.reserve(m_LoadedByClient.size()); + for (auto ClientHandle : m_LoadedByClient) { - currentPlayer = (*itr)->GetPlayer(); - playerPositions.push_back(&(currentPlayer->GetPosition())); + const cPlayer * currentPlayer = ClientHandle->GetPlayer(); + PlayerPositions.push_back(currentPlayer->GetPosition()); } Vector3d currentPosition; - for (auto itr = m_Entities.begin(); itr != m_Entities.end(); ++itr) + for (auto entity : m_Entities) { // LOGD("Counting entity #%i (%s)", (*itr)->GetUniqueID(), (*itr)->GetClass()); - if ((*itr)->IsMob()) + if (entity->IsMob()) { - auto & Monster = reinterpret_cast(**itr); + auto & Monster = reinterpret_cast(*entity); currentPosition = Monster.GetPosition(); - for (auto itr2 = playerPositions.cbegin(); itr2 != playerPositions.cend(); ++itr2) + for (const auto PlayerPos : PlayerPositions) { - toFill.CollectMob(Monster, *this, (currentPosition - **itr2).SqrLength()); + toFill.CollectMob(Monster, *this, (currentPosition - PlayerPos).SqrLength()); } } } // for itr - m_Entitites[] @@ -779,17 +779,17 @@ void cChunk::BroadcastPendingBlockChanges(void) if (m_PendingSendBlocks.size() >= 10240) { // Resend the full chunk - for (cClientHandleList::iterator itr = m_LoadedByClient.begin(), end = m_LoadedByClient.end(); itr != end; ++itr) + for (auto ClientHandle : m_LoadedByClient) { - m_World->ForceSendChunkTo(m_PosX, m_PosZ, cChunkSender::E_CHUNK_PRIORITY_MEDIUM, (*itr)); + m_World->ForceSendChunkTo(m_PosX, m_PosZ, cChunkSender::E_CHUNK_PRIORITY_MEDIUM, ClientHandle); } } else { // Only send block changes - for (cClientHandleList::iterator itr = m_LoadedByClient.begin(), end = m_LoadedByClient.end(); itr != end; ++itr) + for (auto ClientHandle : m_LoadedByClient) { - (*itr)->SendBlockChanges(m_PosX, m_PosZ, m_PendingSendBlocks); + ClientHandle->SendBlockChanges(m_PosX, m_PosZ, m_PendingSendBlocks); } } m_PendingSendBlocks.clear(); @@ -1758,9 +1758,9 @@ void cChunk::SetAreaBiome(int a_MinRelX, int a_MaxRelX, int a_MinRelZ, int a_Max MarkDirty(); // Re-send the chunk to all clients: - for (cClientHandleList::iterator itr = m_LoadedByClient.begin(); itr != m_LoadedByClient.end(); ++itr) + for (auto ClientHandle : m_LoadedByClient) { - m_World->ForceSendChunkTo(m_PosX, m_PosZ, cChunkSender::E_CHUNK_PRIORITY_MEDIUM, (*itr)); + m_World->ForceSendChunkTo(m_PosX, m_PosZ, cChunkSender::E_CHUNK_PRIORITY_MEDIUM, ClientHandle); } // for itr - m_LoadedByClient[] } @@ -1856,15 +1856,13 @@ void cChunk::RemoveBlockEntity(cBlockEntity * a_BlockEntity) bool cChunk::AddClient(cClientHandle * a_Client) { - for (cClientHandleList::iterator itr = m_LoadedByClient.begin(); itr != m_LoadedByClient.end(); ++itr) + if (std::find(m_LoadedByClient.begin(), m_LoadedByClient.end(), a_Client) != m_LoadedByClient.end()) { - if (a_Client == *itr) - { - // Already there, nothing needed - return false; - } + // Already there, nothing needed + return false; } - m_LoadedByClient.push_back( a_Client); + + m_LoadedByClient.push_back(a_Client); for (cEntityList::iterator itr = m_Entities.begin(); itr != m_Entities.end(); ++itr) { @@ -1887,31 +1885,24 @@ bool cChunk::AddClient(cClientHandle * a_Client) void cChunk::RemoveClient(cClientHandle * a_Client) { - for (cClientHandleList::iterator itrC = m_LoadedByClient.begin(); itrC != m_LoadedByClient.end(); ++itrC) + std::remove(m_LoadedByClient.begin(), m_LoadedByClient.end(), a_Client); + + if (!a_Client->IsDestroyed()) { - if (*itrC != a_Client) + for (auto Entity : m_Entities) { - continue; + /* + // DEBUG: + LOGD("chunk [%i, %i] destroying entity #%i for player \"%s\"", + m_PosX, m_PosZ, + (*itr)->GetUniqueID(), a_Client->GetUsername().c_str() + ); + */ + a_Client->SendDestroyEntity(*Entity); } + } - m_LoadedByClient.erase(itrC); - - if (!a_Client->IsDestroyed()) - { - for (cEntityList::iterator itrE = m_Entities.begin(); itrE != m_Entities.end(); ++itrE) - { - /* - // DEBUG: - LOGD("chunk [%i, %i] destroying entity #%i for player \"%s\"", - m_PosX, m_PosZ, - (*itr)->GetUniqueID(), a_Client->GetUsername().c_str() - ); - */ - a_Client->SendDestroyEntity(*(*itrE)); - } - } - return; - } // for itr - m_LoadedByClient[] + return; } @@ -1920,14 +1911,7 @@ void cChunk::RemoveClient(cClientHandle * a_Client) bool cChunk::HasClient(cClientHandle * a_Client) { - for (cClientHandleList::const_iterator itr = m_LoadedByClient.begin(); itr != m_LoadedByClient.end(); ++itr) - { - if ((*itr) == a_Client) - { - return true; - } - } - return false; + return std::find(m_LoadedByClient.begin(), m_LoadedByClient.end(), a_Client) != m_LoadedByClient.end(); } @@ -2793,9 +2777,9 @@ cChunk * cChunk::GetRelNeighborChunkAdjustCoords(int & a_RelX, int & a_RelZ) con void cChunk::BroadcastAttachEntity(const cEntity & a_Entity, const cEntity * a_Vehicle) { - for (cClientHandleList::const_iterator itr = m_LoadedByClient.begin(); itr != m_LoadedByClient.end(); ++itr) + for (auto ClientHandle : m_LoadedByClient) { - (*itr)->SendAttachEntity(a_Entity, a_Vehicle); + ClientHandle->SendAttachEntity(a_Entity, a_Vehicle); } // for itr - LoadedByClient[] } @@ -2805,7 +2789,7 @@ void cChunk::BroadcastAttachEntity(const cEntity & a_Entity, const cEntity * a_V void cChunk::BroadcastBlockAction(int a_BlockX, int a_BlockY, int a_BlockZ, char a_Byte1, char a_Byte2, BLOCKTYPE a_BlockType, const cClientHandle * a_Exclude) { - for (cClientHandleList::const_iterator itr = m_LoadedByClient.begin(); itr != m_LoadedByClient.end(); ++itr) + for (auto itr = m_LoadedByClient.begin(); itr != m_LoadedByClient.end(); ++itr) { if (*itr == a_Exclude) { @@ -2821,7 +2805,7 @@ void cChunk::BroadcastBlockAction(int a_BlockX, int a_BlockY, int a_BlockZ, char void cChunk::BroadcastBlockBreakAnimation(UInt32 a_EntityID, int a_BlockX, int a_BlockY, int a_BlockZ, char a_Stage, const cClientHandle * a_Exclude) { - for (cClientHandleList::iterator itr = m_LoadedByClient.begin(); itr != m_LoadedByClient.end(); ++itr) + for (auto itr = m_LoadedByClient.begin(); itr != m_LoadedByClient.end(); ++itr) { if (*itr == a_Exclude) { @@ -2843,7 +2827,7 @@ void cChunk::BroadcastBlockEntity(int a_BlockX, int a_BlockY, int a_BlockZ, cons { return; } - for (cClientHandleList::iterator itr = m_LoadedByClient.begin(); itr != m_LoadedByClient.end(); ++itr) + for (auto itr = m_LoadedByClient.begin(); itr != m_LoadedByClient.end(); ++itr) { if (*itr == a_Exclude) { @@ -2859,7 +2843,7 @@ void cChunk::BroadcastBlockEntity(int a_BlockX, int a_BlockY, int a_BlockZ, cons void cChunk::BroadcastCollectEntity(const cEntity & a_Entity, const cPlayer & a_Player, const cClientHandle * a_Exclude) { - for (cClientHandleList::iterator itr = m_LoadedByClient.begin(); itr != m_LoadedByClient.end(); ++itr) + for (auto itr = m_LoadedByClient.begin(); itr != m_LoadedByClient.end(); ++itr) { if (*itr == a_Exclude) { @@ -2875,7 +2859,7 @@ void cChunk::BroadcastCollectEntity(const cEntity & a_Entity, const cPlayer & a_ void cChunk::BroadcastDestroyEntity(const cEntity & a_Entity, const cClientHandle * a_Exclude) { - for (cClientHandleList::const_iterator itr = m_LoadedByClient.begin(); itr != m_LoadedByClient.end(); ++itr) + for (auto itr = m_LoadedByClient.begin(); itr != m_LoadedByClient.end(); ++itr) { if (*itr == a_Exclude) { @@ -2891,7 +2875,7 @@ void cChunk::BroadcastDestroyEntity(const cEntity & a_Entity, const cClientHandl void cChunk::BroadcastEntityEffect(const cEntity & a_Entity, int a_EffectID, int a_Amplifier, short a_Duration, const cClientHandle * a_Exclude) { - for (cClientHandleList::const_iterator itr = m_LoadedByClient.begin(); itr != m_LoadedByClient.end(); ++itr) + for (auto itr = m_LoadedByClient.begin(); itr != m_LoadedByClient.end(); ++itr) { if (*itr == a_Exclude) { @@ -2907,7 +2891,7 @@ void cChunk::BroadcastEntityEffect(const cEntity & a_Entity, int a_EffectID, int void cChunk::BroadcastEntityEquipment(const cEntity & a_Entity, short a_SlotNum, const cItem & a_Item, const cClientHandle * a_Exclude) { - for (cClientHandleList::const_iterator itr = m_LoadedByClient.begin(); itr != m_LoadedByClient.end(); ++itr) + for (auto itr = m_LoadedByClient.begin(); itr != m_LoadedByClient.end(); ++itr) { if (*itr == a_Exclude) { @@ -2923,7 +2907,7 @@ void cChunk::BroadcastEntityEquipment(const cEntity & a_Entity, short a_SlotNum, void cChunk::BroadcastEntityHeadLook(const cEntity & a_Entity, const cClientHandle * a_Exclude) { - for (cClientHandleList::const_iterator itr = m_LoadedByClient.begin(); itr != m_LoadedByClient.end(); ++itr) + for (auto itr = m_LoadedByClient.begin(); itr != m_LoadedByClient.end(); ++itr) { if (*itr == a_Exclude) { @@ -2939,7 +2923,7 @@ void cChunk::BroadcastEntityHeadLook(const cEntity & a_Entity, const cClientHand void cChunk::BroadcastEntityLook(const cEntity & a_Entity, const cClientHandle * a_Exclude) { - for (cClientHandleList::const_iterator itr = m_LoadedByClient.begin(); itr != m_LoadedByClient.end(); ++itr) + for (auto itr = m_LoadedByClient.begin(); itr != m_LoadedByClient.end(); ++itr) { if (*itr == a_Exclude) { @@ -2955,7 +2939,7 @@ void cChunk::BroadcastEntityLook(const cEntity & a_Entity, const cClientHandle * void cChunk::BroadcastEntityMetadata(const cEntity & a_Entity, const cClientHandle * a_Exclude) { - for (cClientHandleList::const_iterator itr = m_LoadedByClient.begin(); itr != m_LoadedByClient.end(); ++itr) + for (auto itr = m_LoadedByClient.begin(); itr != m_LoadedByClient.end(); ++itr) { if (*itr == a_Exclude) { @@ -2971,7 +2955,7 @@ void cChunk::BroadcastEntityMetadata(const cEntity & a_Entity, const cClientHand void cChunk::BroadcastEntityRelMove(const cEntity & a_Entity, char a_RelX, char a_RelY, char a_RelZ, const cClientHandle * a_Exclude) { - for (cClientHandleList::const_iterator itr = m_LoadedByClient.begin(); itr != m_LoadedByClient.end(); ++itr) + for (auto itr = m_LoadedByClient.begin(); itr != m_LoadedByClient.end(); ++itr) { if (*itr == a_Exclude) { @@ -2987,7 +2971,7 @@ void cChunk::BroadcastEntityRelMove(const cEntity & a_Entity, char a_RelX, char void cChunk::BroadcastEntityRelMoveLook(const cEntity & a_Entity, char a_RelX, char a_RelY, char a_RelZ, const cClientHandle * a_Exclude) { - for (cClientHandleList::const_iterator itr = m_LoadedByClient.begin(); itr != m_LoadedByClient.end(); ++itr) + for (auto itr = m_LoadedByClient.begin(); itr != m_LoadedByClient.end(); ++itr) { if (*itr == a_Exclude) { @@ -3003,7 +2987,7 @@ void cChunk::BroadcastEntityRelMoveLook(const cEntity & a_Entity, char a_RelX, c void cChunk::BroadcastEntityStatus(const cEntity & a_Entity, char a_Status, const cClientHandle * a_Exclude) { - for (cClientHandleList::const_iterator itr = m_LoadedByClient.begin(); itr != m_LoadedByClient.end(); ++itr) + for (auto itr = m_LoadedByClient.begin(); itr != m_LoadedByClient.end(); ++itr) { if (*itr == a_Exclude) { @@ -3019,7 +3003,7 @@ void cChunk::BroadcastEntityStatus(const cEntity & a_Entity, char a_Status, cons void cChunk::BroadcastEntityVelocity(const cEntity & a_Entity, const cClientHandle * a_Exclude) { - for (cClientHandleList::const_iterator itr = m_LoadedByClient.begin(); itr != m_LoadedByClient.end(); ++itr) + for (auto itr = m_LoadedByClient.begin(); itr != m_LoadedByClient.end(); ++itr) { if (*itr == a_Exclude) { @@ -3035,7 +3019,7 @@ void cChunk::BroadcastEntityVelocity(const cEntity & a_Entity, const cClientHand void cChunk::BroadcastEntityAnimation(const cEntity & a_Entity, char a_Animation, const cClientHandle * a_Exclude) { - for (cClientHandleList::const_iterator itr = m_LoadedByClient.begin(); itr != m_LoadedByClient.end(); ++itr) + for (auto itr = m_LoadedByClient.begin(); itr != m_LoadedByClient.end(); ++itr) { if (*itr == a_Exclude) { @@ -3051,7 +3035,7 @@ void cChunk::BroadcastEntityAnimation(const cEntity & a_Entity, char a_Animation void cChunk::BroadcastParticleEffect(const AString & a_ParticleName, float a_SrcX, float a_SrcY, float a_SrcZ, float a_OffsetX, float a_OffsetY, float a_OffsetZ, float a_ParticleData, int a_ParticleAmount, cClientHandle * a_Exclude) { - for (cClientHandleList::iterator itr = m_LoadedByClient.begin(); itr != m_LoadedByClient.end(); ++itr) + for (auto itr = m_LoadedByClient.begin(); itr != m_LoadedByClient.end(); ++itr) { if (*itr == a_Exclude) { @@ -3067,7 +3051,7 @@ void cChunk::BroadcastParticleEffect(const AString & a_ParticleName, float a_Src void cChunk::BroadcastRemoveEntityEffect(const cEntity & a_Entity, int a_EffectID, const cClientHandle * a_Exclude) { - for (cClientHandleList::iterator itr = m_LoadedByClient.begin(); itr != m_LoadedByClient.end(); ++itr) + for (auto itr = m_LoadedByClient.begin(); itr != m_LoadedByClient.end(); ++itr) { if (*itr == a_Exclude) { @@ -3083,7 +3067,7 @@ void cChunk::BroadcastRemoveEntityEffect(const cEntity & a_Entity, int a_EffectI void cChunk::BroadcastSoundEffect(const AString & a_SoundName, double a_X, double a_Y, double a_Z, float a_Volume, float a_Pitch, const cClientHandle * a_Exclude) { - for (cClientHandleList::iterator itr = m_LoadedByClient.begin(); itr != m_LoadedByClient.end(); ++itr) + for (auto itr = m_LoadedByClient.begin(); itr != m_LoadedByClient.end(); ++itr) { if (*itr == a_Exclude) { @@ -3099,7 +3083,7 @@ void cChunk::BroadcastSoundEffect(const AString & a_SoundName, double a_X, doubl void cChunk::BroadcastSoundParticleEffect(int a_EffectID, int a_SrcX, int a_SrcY, int a_SrcZ, int a_Data, const cClientHandle * a_Exclude) { - for (cClientHandleList::iterator itr = m_LoadedByClient.begin(); itr != m_LoadedByClient.end(); ++itr) + for (auto itr = m_LoadedByClient.begin(); itr != m_LoadedByClient.end(); ++itr) { if (*itr == a_Exclude) { @@ -3115,7 +3099,7 @@ void cChunk::BroadcastSoundParticleEffect(int a_EffectID, int a_SrcX, int a_SrcY void cChunk::BroadcastSpawnEntity(cEntity & a_Entity, const cClientHandle * a_Exclude) { - for (cClientHandleList::iterator itr = m_LoadedByClient.begin(); itr != m_LoadedByClient.end(); ++itr) + for (auto itr = m_LoadedByClient.begin(); itr != m_LoadedByClient.end(); ++itr) { if (*itr == a_Exclude) { @@ -3131,7 +3115,7 @@ void cChunk::BroadcastSpawnEntity(cEntity & a_Entity, const cClientHandle * a_Ex void cChunk::BroadcastThunderbolt(int a_BlockX, int a_BlockY, int a_BlockZ, const cClientHandle * a_Exclude) { - for (cClientHandleList::iterator itr = m_LoadedByClient.begin(); itr != m_LoadedByClient.end(); ++itr) + for (auto itr = m_LoadedByClient.begin(); itr != m_LoadedByClient.end(); ++itr) { if (*itr == a_Exclude) { @@ -3147,7 +3131,7 @@ void cChunk::BroadcastThunderbolt(int a_BlockX, int a_BlockY, int a_BlockZ, cons void cChunk::BroadcastUseBed(const cEntity & a_Entity, int a_BlockX, int a_BlockY, int a_BlockZ) { - for (cClientHandleList::iterator itr = m_LoadedByClient.begin(); itr != m_LoadedByClient.end(); ++itr) + for (auto itr = m_LoadedByClient.begin(); itr != m_LoadedByClient.end(); ++itr) { (*itr)->SendUseBed(a_Entity, a_BlockX, a_BlockY, a_BlockZ); } // for itr - LoadedByClient[] diff --git a/src/Chunk.h b/src/Chunk.h index fd9ea0b0c..683965c62 100644 --- a/src/Chunk.h +++ b/src/Chunk.h @@ -439,7 +439,12 @@ public: void SetAlwaysTicked(bool a_AlwaysTicked); // Makes a copy of the list - cClientHandleList GetAllClients(void) const {return m_LoadedByClient; } + cClientHandleList GetAllClients(void) const + { + cClientHandleList copy; + std::copy(m_LoadedByClient.begin(), m_LoadedByClient.end(), std::back_inserter(copy)); + return copy; + } private: @@ -479,9 +484,9 @@ private: sSetBlockQueueVector m_SetBlockQueue; ///< Block changes that are queued to a specific tick // A critical section is not needed, because all chunk access is protected by its parent ChunkMap's csLayers - cClientHandleList m_LoadedByClient; - cEntityList m_Entities; - cBlockEntityList m_BlockEntities; + std::vector m_LoadedByClient; + cEntityList m_Entities; + cBlockEntityList m_BlockEntities; /** Number of times the chunk has been requested to stay (by various cChunkStay objects); if zero, the chunk can be unloaded */ int m_StayCount; -- cgit v1.2.3