Reduce lag spikes from HarvesterBotModule.#21471
Conversation
|
Quick review, LGTM although a careful observer would be able to tell if an ore patch has enemies on it. |
RoosterDragon
force-pushed
the
ai-harv-perf
branch
2 times, most recently
from
6fa0cff to
1145bd3
Compare
|
I think this workaround cannot fix the key problem of HarvesterBotModule: On the worst case, it will go through the entire map to find a place to harvest. |
RoosterDragon
force-pushed
the
ai-harv-perf
branch
from
1145bd3 to
93446cf
Compare
You're right. I've pushed a commit which uses a proper path search instead. This improves the situation even further - and revealed a bonus pathfinding bug which I've fixed in order to make it work. Using the same test map, the perf graph is now even less affected by the bot_tick timing.
|
This comment was marked as resolved.
This comment was marked as resolved.
|
With shroud disabled - or when explored - would a harvester drive all the way across the map to possibly harvest ore next to an enemy harvester? Just curious. |
|
Could not lead to starvation of harvester A because B each time find the ore earlier? What if A and B belong to different teams. |
|
Yes, currently the logic puts no limit on the distance. So if there's no nearby ore patches, it is allowed to drive across the map to find some. It prefers ore patches without enemies, but that's only a preference. So if there's only one ore patch available and it has enemies nearby, it will still attempt to go there. But primarily it wants a nearby ore patch, so it won't go driving across the map until it's used up the nearby ones. |
PunkPun
left a comment
PunkPun
left a comment
There was a problem hiding this comment.
The AI harvester's never respected the rule that they should choose ore that's closer to the refinery. As this is changing the heuteristic could that be addressed as well?
On bleed this logic seems to be broken even for non-ai players though. Dunno when it regressed
|
This AI logic scans from the havester location, rather than the refinery. But you mention this being broken for players too? That sounds like it's something different. This system is for AI players only, to notice idle harvesters and get them doing something. It's not related to the automated things a harvester for both a human and AI player would do as it carries out orders. Could you link any relevant tickets? I can't tell whether this is something we should tweak in this PR or come back to separately. |
|
PunkPun may also refer to what i described in #20058, for players yes. |
RoosterDragon
force-pushed
the
ai-harv-perf
branch
from
93446cf to
cd8351c
Compare
|
I've pushed an additional commit that changes the behaviour, for AI players only, to locate ore closest to a refinery, rather than the harvester's location. |
|
Isn't it odd that harvesters of AI players would use other logic than of normal players to find ore? Couldn't it be better fixed in a central location/one-level-up? A normal player also doesn't often direct a harvester? Not sure. |
|
The behaviour of this bot module is only for idle harvesters of the AI player, it doesn't affect the automated decisions made by the harvester itself. So if a human players sees an idle harvester, they will need to order it after some ore themselves. This bot module is doing the equivalent for an AI player. Once the harvester is harvesting, this bot module is hands off after that, until they go idle again. |
|
Again, this logic only applies to idle harvesters - once they're already harvesting this bot module does not touch them. That's the existing behaviour. My scope for the PR was to fix the performance of the existing logic. If you want different logic, please do raise a separate issue for that. I wish to keep any fundamental behaviour changes out of scope for this PR. |
|
hmm, that makes it very hard to test. Harvesters almost never idle |
|
The PR description has a test map. |
When this hits the case that "As both ends are accessible, we can freely swap them." - we must note that we are reversing the path search and pass the information into the HierarchicalPathFinder. When a normal path search occurs, the actor trying to pathfind will never check its own location - and thus never gets blocked by itself. When a search is reversed, the search will check the actors location. If we inform the search it is doing done in reverse, it will special case this scenario and avoid the actor blocking itself. But if it is not told about this scenario, then this special case is not applied and no path will be found when in fact a path is possible.
The AI uses HarvesterBotModule to check for idle harvesters, and give them harvest orders. By default it scans every 50 ticks (2 seconds at normal speed), and for any idle harvesters locates an ore patch and issues a harvest order. This FindNextResource to scan for a suitable ore path is quite expensive. If the AI has to scan for ore patches for several harvesters, then this can produce a noticeable lag spike. Additionally, when there are no available ore patches, the scan will just keep repeating since the harvesters will always be idle - thus the lag spikes repeat every 50 ticks. To reduce the impact, there already exists a randomization on the first scan interval so that multiple different AIs scan on different ticks. By ensuring the AI players scan at different times, we avoid a huge lag spike where they all operate on the same tick. To reduce the impact even more, we make four additional changes: - Scans continue to be done every 50 ticks to detect harvesters. But we spread out the searches for ore patches over multiple later ticks. We'll only perform one ore patch search per tick. This means instead of ordering e.g. 30 harvesters on a single tick and creating a spike, we order one on each tick over the next 30 ticks instead. This spreads out the performance impact. - When a harvester fails to locate any suitable ore patch, we put it on a longer cooldown, by default 5x the regular cooldown. We don't need to scan as often for these harvesters, since it'll take time for new resources to appear. - We change the path search in FindNextResource from FindPathToTargetCellByPredicate to FindPathToTargetCells. The format in an undirected path search that must flood fill from the start location. If ore is on the other side of the map, this entails searching the whole map which is very expensive. By maintaining a lookup of resource types per cell, we can instead give the target locations directly to the path search. This lookup requires a small overhead to maintain, but allows for a far more efficient path search to be carried out. The search can be directed towards the target locations, and the hierarchical path finder can be employed resulting in a path search that explores far fewer cells. A few tweaks are made to ResourceClaimLayer to avoid it creating empty list entries when this can be avoided. - We adjust how the enemy avoidance cost is done. Previously, this search used world.FindActorsInCircle to check for nearby enemies, but this check was done for every cell that was searched, and is itself quite expensive. Now, we create a series of "bins" and cache the additional cost for that bin. This is a less fine grained approach but is sufficient for our intended goal of "avoid resource patches with too many enemies nearby". The customCost function is now less expensive so we can reuse the avoidance cost stored for each bin, rather than calculating fresh for every cell.
When HarvesterBotModule is ordering idle harvesters to nearby resources, it previously scanned from the harvester's current location. Instead, it now scans from the location of the nearest refinery. As the harvester will likely make many runs between the resource and the refinery, it is better to choose a location near the refinery. This will minimise overall distance travelled to harvest the resource patch.
RoosterDragon
force-pushed
the
ai-harv-perf
branch
from
cd8351c to
4633281
Compare
Mailaender
left a comment
Mailaender
left a comment
There was a problem hiding this comment.
I can't spot regressions in-game. Path finding seems effective. Thread avoidance seems to work, but it is not flawless.
6 participants
Reduce lag spikes from HarvesterBotModule.
The AI uses HarvesterBotModule to check for idle harvesters, and give them harvest orders. By default it scans every 50 ticks (2 seconds at normal speed), and for any idle harvesters locates an ore patch and issues a harvest order. This FindNextResource to scan for a suitable ore path is quite expensive. If the AI has to scan for ore patches for several harvesters, then this can produce a noticeable lag spike. Additionally, when there are no available ore patches, the scan will just keep repeating since the harvesters will always be idle - thus the lag spikes repeat every 50 ticks.
To reduce the impact, there already exists a randomization on the first scan interval so that multiple different AIs scan on different ticks. By ensuring the AI players scan at different times, we avoid a huge lag spike where they all operate on the same tick.
To reduce the impact even more, we make four additional changes:
Fixes #18910. Fixes #16163 (no longer required now the perf is fixed)
Test map:
ai-harv-perf-test.zip
Spawn an AI in spawn A.
Bleed: A huge lag spike occurs when the map is started, and the AI orders the harvesters all at once.
PR (before improvement): A small impact persists for several ticks on the performance graph when the map starts, but no lag spike.
PR (current): The impact of bot_tick is now almost entirly removed.
The test map also shows harvesters avoiding the ore patch with many enemies, but some are willing to go to an area with light enemy presence rather than going too far.