Implement a new container to replace most of our std::maps in ComponentManager.
This also replaces EntityMap in the RangeManager.
Performance characteristics are "fast everything but very fast iteration".
Differential D1739
Introduce a replacement container for EntityMap, std::map, std::unordered_map and boost::unordered_map wraitii on Jan 7 2019, 7:14 PM. Authored by
Details
Implement a new container to replace most of our std::maps in ComponentManager. Performance characteristics are "fast everything but very fast iteration". Code review. Performance review. Gameplay profiling. Performance testing can be done off-line through this repo: https://github.com/wraitii/entities_temp
Diff Detail
Event TimelineThere are a very large number of changes, so older changes are hidden. Show Older Changes Comment Actions Successful build - Chance fights ever on the side of the prudent. Link to build: https://jenkins.wildfiregames.com/job/differential/938/ Comment Actions Might as well make this revision useful. BOOM. Massive SFINAE for this new container. I need to assess performance exactly as testing results have proven rather inconclusive. A short AI game (30 minutes) would indicate this is slightly faster than svn, but I'd like longer replays and more profiling. Furthermore, this might still be optimised. This works so long as you use D1736. I've tested that the hashes are the same, and the tests have become quite extensive so I'm rather confident that it works, the question is "how fast". Comment Actions Build failure - The Moirai have given mortals hearts that can endure. Linter detected issues: Executing section Source... source/simulation2/Simulation2.h | 1| /*·Copyright·(C)·2018·Wildfire·Games. | | [NORMAL] LicenseYearBear: | | License should have "2019" year instead of "2018" source/simulation2/system/ComponentManager.h | 1| /*·Copyright·(C)·2018·Wildfire·Games. | | [NORMAL] LicenseYearBear: | | License should have "2019" year instead of "2018" source/simulation2/system/ComponentManagerSerialization.cpp | 1| /*·Copyright·(C)·2017·Wildfire·Games. | | [NORMAL] LicenseYearBear: | | License should have "2019" year instead of "2017" source/tools/atlas/GameInterface/DeltaArray.h | 1| /*·Copyright·(C)·2009·Wildfire·Games. | | [NORMAL] LicenseYearBear: | | License should have "2019" year instead of "2009" source/simulation2/system/ComponentManager.cpp | 1| /*·Copyright·(C)·2018·Wildfire·Games. | | [NORMAL] LicenseYearBear: | | License should have "2019" year instead of "2018" Executing section JS... Executing section cli... Link to build: https://jenkins.wildfiregames.com/job/differential/961/ Comment Actions After much performance testing, I've found that this variant using a vector and a deque is probably the most efficient as a map replacement, as it performs extremely well for iteration and only about 3x as slowly as our current EntityMap for other operations (which remains faster than unordered_map). The code is also smaller and neater. The trouble is that this changes hashes because the order, while deterministic, is no longer sorted by ID when iterating. Thus I can't test so straightforwardly. I'll try to get an idea of the overall effect on actual game results regardless. Comment Actions Build failure - The Moirai have given mortals hearts that can endure. Linter detected issues: Executing section Source... source/simulation2/system/ComponentManagerSerialization.cpp | 1| /*·Copyright·(C)·2017·Wildfire·Games. | | [NORMAL] LicenseYearBear: | | License should have "2019" year instead of "2017" source/simulation2/system/ComponentManager.h | 1| /*·Copyright·(C)·2018·Wildfire·Games. | | [NORMAL] LicenseYearBear: | | License should have "2019" year instead of "2018" source/simulation2/helpers/Selection.cpp | 1| /*·Copyright·(C)·2017·Wildfire·Games. | | [NORMAL] LicenseYearBear: | | License should have "2019" year instead of "2017" source/simulation2/Simulation2.h | 1| /*·Copyright·(C)·2018·Wildfire·Games. | | [NORMAL] LicenseYearBear: | | License should have "2019" year instead of "2018" source/simulation2/system/Component.h | 1| /*·Copyright·(C)·2017·Wildfire·Games. | | [NORMAL] LicenseYearBear: | | License should have "2019" year instead of "2017" source/simulation2/system/ComponentManager.cpp | 1| /*·Copyright·(C)·2018·Wildfire·Games. | | [NORMAL] LicenseYearBear: | | License should have "2019" year instead of "2018" source/graphics/ObjectManager.cpp | 1| /*·Copyright·(C)·2015·Wildfire·Games. | | [NORMAL] LicenseYearBear: | | License should have "2019" year instead of "2015" source/tools/atlas/GameInterface/DeltaArray.h | 1| /*·Copyright·(C)·2009·Wildfire·Games. | | [NORMAL] LicenseYearBear: | | License should have "2019" year instead of "2009" source/tools/atlas/GameInterface/Handlers/ObjectHandlers.cpp | 33| #include·"graphics/Terrain.h" | | [MAJOR] CPPCheckBear (syntaxError): | | Invalid number of character ({) when these macros are defined: ''. source/tools/atlas/GameInterface/Handlers/ObjectHandlers.cpp | 33| #include·"graphics/Terrain.h" | | [MAJOR] CPPCheckBear (syntaxError): | | Invalid number of character ({) when these macros are defined: 'MESSAGES_SKIP_STRUCTS'. source/tools/atlas/GameInterface/Handlers/ObjectHandlers.cpp | 33| #include·"graphics/Terrain.h" | | [MAJOR] CPPCheckBear (syntaxError): | | Invalid number of character ({) when these macros are defined: '_MSC_VER'. Executing section JS... Executing section cli... Link to build: https://jenkins.wildfiregames.com/job/differential/988/ Comment Actions
It's faster at iteration, so it might work out. Might also not. Hard to tell really. Comment Actions This really needs to be used for components before I decide if it's worth merging, but as it changes hashes it's kind of annoying. I'll probably have to do some fancy tests at some point. Comment Actions It wasn't because the former version was just a rewrite, this is a rather large behaviour change that makes some of the existing code slower and some faster (hopefully overall the result is faster though).
Sure If I commit it without a review since it's got no comments/reviews.
Comment Actions @fabio would be happy to add boost references if it really improves performance?
Comment Actions Quick rebase, and also replace the std::map for static/unit shapes. Been trying some "overall" profiling to no real avail on some contained cases such combat demonstrations (huge). I think the raw overhead is somewhat dwarfed by the other costs. Comment Actions Build failure - The Moirai have given mortals hearts that can endure. Link to build: https://jenkins.wildfiregames.com/job/docker-differential/304/display/redirect Comment Actions (only some linting)
Comment Actions I'll do a pass over the autos' and the code. Performance wise, except for inserting/deleting which is about 3x worse, I've again tested this to be rather similar to EntityMap, better at iterating when sparsity becomes a factor of 2 or 3... I need to check what we realistically get in-game. I think I could do an optimisation for when pointers are stored which might actually be interesting for the component manager.
Comment Actions I've done much profiling, and much testing, and here's a CRTP-ed version (comes in single key and dual-key mode). It's at least as good as unordered_map, often-times much better, and I've ran some custom scenarios where I saw almost 10% increase in speed. Haven't changed m_ComponentCaches (yet) as my testing showed unordered_map having similar perf to DualFlatMap. Can be tested later. This introduces a side-effect change that subscriptions now keep pointers to entity-component maps, instead of going through an intermediate find, which should be a slight speedup. Comment Actions Build failure - The Moirai have given mortals hearts that can endure. Link to build: https://jenkins.wildfiregames.com/job/docker-differential/355/display/redirect Comment Actions Ran a few rejointests, all passing, which indicates that the serialisation keeps the deterministic non-linear order correctly, so this is good to go. Comment Actions Feels like the above was a somewhat pointless exercise, TBH. See D3186 for an actual improvement on the component front. We could probably do with replacing some std::map with flat maps in the componentManager for stuff that indexes by interfaceID / component ID, but that's not going to be a huge improvement. Rewriting EntityMap itself is probably still worthwhile though. |