-
Notifications
You must be signed in to change notification settings - Fork 151
perf(particlesys): Reduce cost of ParticleSystemManager::findParticleSystem() by 80% #2217
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Greptile Overview
|
| Filename | Overview |
|---|---|
| Core/GameEngine/Include/Common/STLTypedefs.h | Added ParticleSystemID forward declaration and hash specialization for use in hash maps |
| Core/GameEngine/Include/GameClient/ParticleSys.h | Added ParticleSystemIDMap hash map member to enable O(1) lookups by particle system ID |
| Core/GameEngine/Source/GameClient/System/ParticleSys.cpp | Refactored findParticleSystem() to use hash map, updated add/remove operations to maintain hash map sync, added assertion for validation |
Sequence Diagram
sequenceDiagram
participant Client
participant PSM as ParticleSystemManager
participant List as m_allParticleSystemList
participant Map as m_systemMap (new)
Note over PSM,Map: Add Operation
Client->>PSM: friend_addParticleSystem(ps)
PSM->>List: push_back(ps)
PSM->>Map: m_systemMap[ps->getSystemID()] = ps
PSM->>PSM: ++m_particleSystemCount
Note over PSM,Map: Find Operation (optimized)
Client->>PSM: findParticleSystem(id)
alt id == INVALID_PARTICLE_SYSTEM_ID
PSM-->>Client: return nullptr
else valid id
PSM->>Map: find(id)
Map-->>PSM: iterator
PSM-->>Client: return it->second or nullptr
end
Note over PSM,Map: Remove Operation
Client->>PSM: friend_removeParticleSystem(ps)
PSM->>List: find(ps)
PSM->>Map: erase(ps->getSystemID())
PSM->>List: erase(it)
PSM->>PSM: --m_particleSystemCount
| DEBUG_ASSERTCRASH(particleSystemToAdd != nullptr, ("ParticleSystemManager::friend_addParticleSystem: ParticleSystem is null")); | ||
| m_allParticleSystemList.push_back(particleSystemToAdd); | ||
| m_systemMap[particleSystemToAdd->getSystemID()] = particleSystemToAdd; | ||
| ++m_particleSystemCount; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do STLport hash maps store the container size?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes they do. Modern std::list also does :)
|
Fixed nasty VC6 compile error |
This change reduces the cost of
ParticleSystemManager::findParticleSystem()by 80% using a hash map instead of linear search through thestd::list.Tested in Release with 100'000 iterations on a big test map, which took around 30 seconds to hit.
The ParticleSystem count was moderate around 700. The cost was reduced by around 80% in the test.
Original
100'000 iterations measured at 87 ms
This change
100'000 iterations measured at 18 ms
In unoptimized builds the performance gain will be even larger.
Out of
addParticleSystem,removeParticleSystemandfindParticleSystem, theremoveParticleSystemshows to be the most expensive, because it still has astd::listsearch in it, naturally.