Getting the attack part of the entity state alone is already quite expensive (on the order of >5 ms with 200 units selected), and it is unused in cases where multiple units are selected.
Therefore, bring back something resembling the earlier divide between GetEntityState, GetExtendedEntityState. I thought quite long about this issue (well, actually, really long), but while being a hacky performance fix, it is most easiest to implement and most other performance improvements would require significant rewrites to the entity system (which might still better be done at some point in the future, but is not of great priority). In measurements on my own computer with the simple F11 profiler it shows that the performance improvement with 200 units selected on Combat Demo (Huge) is about ~30% (~30->20ms for gui sim update). With some additional work in the future some more performance improvements can be made here. An additional point is that there may be some minor overhead from deepfreeze (but I'm unsure about that), an option might be to disable those in release versions, as elexis noted in IRC.
Details
- Reviewers
mimo - Group Reviewers
Restricted Owners Package (Owns No Changed Paths)
- Verify that the game still works correctly (especially when units are selected) (no warnings are displayed).
- Verify in the code that the necessary entity state is available when it is used (because in principle nothing changed to the state returned by GetEntityState, only the state returned from occurences of GetBasicEntityState need to be checked).
Diff Detail
- Repository
- rP 0 A.D. Public Repository
- Branch
- selection
- Lint
Lint OK - Unit
No Unit Test Coverage - Build Status
Buildable 5182 Build 8859: Vulcan Build Jenkins Build 8858: arc lint + arc unit
Event Timeline
Before going back to such a separation (which improved performance, but revealed itself hard to maintain in the long term), it would be good to know exactly where is the real gain: is it because we have less info to transfer? or because we have less info to compute?
If the latter is dominant, it may be more efficient to cache all these attack/armour infos instead of recomputing them every turn (applyValuesModifications is not specially fast) as is done for example in ResourceGatherer for the rates inside OnValueModification (note that this could be done better in ResourceGatherer as everything is recomputed with any change). Doing it that way could also help when fighting as also the simulation would not have to recompute all quantities each time.
I think it's a bit of both, for attack in particular it's mostly the computation time but there is also some transfer cost. if we look at the entire time saved by this patch on my computer I'd estimate 6-7ms is in computation time, 3-4 ms in transfer time. In principle it's going to be difficult to make the selection really fast, simply because of the way our entity system is designed currently (ideally we'd rewrite it in the future).
Build failure - The Moirai have given mortals hearts that can endure.
Link to build: https://jenkins.wildfiregames.com/job/differential/88/display/redirect
Actually, it seems more likely that you're very right and I was mistaken. I will try to implement this!