Improve BotModule performance. #21360
Conversation
Several parts of bot module logic, often through the AIUtils helper class, will query or count over all actors in the world. This is not a fast operation and the AI tends to repeat it often. Introduce some ActorIndex classes that can maintain an index of actors in the world that match a query based on a mix of actor name, owner or trait. These indexes introduce some overhead to maintain, but allow the queries or counts that bot modules needs to perform to be greatly sped up, as the index means there is a much smaller starting set of actors to consider. This is beneficial to the bot logic as the TraitDictionary index maintained by the world works only in terms of traits and doesn't allow the bot logic to perform a sufficiently selective lookup. This is because the bot logic is usually defined in terms of actor names rather than traits.
| @@ -102,7 +106,6 @@ void IBotTick.BotTick(IBot bot) | |||
| scanForIdleHarvestersTicks = Info.ScanForIdleHarvestersInterval; | |||
|
|
|||
| // Find new harvesters | |||
| // TODO: Look for a more performance-friendly way to update this list | |||
| var newHarvesters = world.ActorsHavingTrait<Harvester>().Where(a => !unitCannotBeOrdered(a) && !harvesters.ContainsKey(a)); | |||
There was a problem hiding this comment.
so why aren't we using the actor index to get these harvesters?
There was a problem hiding this comment.
this whole system is meant for tracking existing harvesters and can be replaced
There was a problem hiding this comment.
The comment here explains it - a index on player alone might not pay for itself. https://github.com/OpenRA/OpenRA/pull/21360/files#diff-35fc4a2486ac8f0e2c2e1f2cefc1906401fd30c12d47e5b3395400e3541272f6R65-R67
I spotted some slowness in a few bot methods which motivated this PR, but this line wasn't a problem. I suspect the Harvester trait is selective enough that it isn't actually all that bad to have this as-is.
There was a problem hiding this comment.
but we already have harvestersIndex in the class?
There was a problem hiding this comment.
That one also filters on Info.HarvesterTypes whereas this one doesn't. And given the desc on that of Leave empty to disable harvester replacement. Currently only needed by harvester replacement system. I don't want to apply it here, as that sounds like it could break any configs that were relying on being able to not specify the HarvesterTypes.
|
Although this PR should not have problem on multiplayer, I think some multiplayer AI tests (2 player ob 2 AI in game) can be necessary to confirm its stability. |
Several parts of bot module logic, often through the AIUtils helper class, will query or count over all actors in the world. This is not a fast operation and the AI tends to repeat it often.
Introduce some ActorIndex classes that can maintain an index of actors in the world that match a query based on a mix of actor name, owner or trait. These indexes introduce some overhead to maintain, but allow the queries or counts that bot modules needs to perform to be greatly sped up, as the index means there is a much smaller starting set of actors to consider. This is beneficial to the bot logic as the TraitDictionary index maintained by the world works only in terms of traits and doesn't allow the bot logic to perform a sufficiently selective lookup. This is because the bot logic is usually defined in terms of actor names rather than traits.