CombatExtended-Continued / CombatExtended

Combat Extended mod for RimWorld
370 stars 249 forks source link

[Bug]: Verb_LaunchProjectileCE.cs method TryFindCEShootLineFromTo terrible performance #2395

Open pogoman opened 1 year ago

pogoman commented 1 year ago

Rimworld version: Latest Combat Extended version: Latest Combat Extended source (Steam, GitHub, etc.): Any Your operating system: Windows 11 Your mod list: CE

Im making this bug now because in the new version its still using the same outdated code. The code in Verb_LaunchProjectileCE.cs is copied from an old version of the Rimworld core assembly and desperately needs to be updated. Running tests with unmodded CE only Rimworld at max raid size with and without this file the performance difference is night and day. Its so bad in fact that in my mod i have a patch to disable CE running it. https://github.com/pogoman/All-Raids-Can-Breach/blob/master/Source/Pogoman/Patches/CE/Verb_LaunchProjectileCE.cs

Expected behavior Similar performance to vanilla.

To reproduce See description, Dubs performance analyser is the easiest way to monitor. Verb_LaunchProjectileCE is called thousands of times and eats the majority of cpu time.

I hereby verify that I have done the following:

pogoman commented 1 year ago

As an addendum, performance is only terrible when avoid grid generation is working and raiders enter the avoid grid. Something inside this function: image is causing canshootthroughcell to fail a lot more than the vanilla check which simply calls: GenSight.LineOfSight(sourceSq, targetLoc, this.caster.Map, true, null, 0, 0))

I think because CE needs to check if the bullet is going to hit friendlies in each cell? i.e. this part: image

Not sure on the best way to optimise without sinking a lot of time into it.

AluminumAlman commented 1 year ago

What's weird is that CanHitCellFromCellIgnoringRange doesn't appear to call anything avoid-grid related, both directly and indirectly.

pogoman commented 1 year ago

Ah apologies should have updated this. I've done far more research into this issue since then and have mitigated it for the most part from my mod. Nothing actually to do with the avoid grid itself, just certain raids where there is a lot of long ranged combat from both sides behind cover, the amount of LOS cell checks become insane, we're talking 10s of millions of calls to GenSightCE.PointsOnLineOfSight with maybe 20 raiders tops. This function is then iterated through calling a predicate and you get the idea; constant stuttering. Here is how i resolved the issue: Verb_LaunchProjectileCE.cs Basically the vanilla LOS check is quite simple and very performant. So what i do is run the vanilla check first, if it fails, then the CE one most often will fail as well, so i return false in the patch prefix and dont bother running TryFindCEShootLineFromTo. If however vanilla TryFindShootLineFromTo passes then i need to run TryFindCEShootLineFromTo as well to ensure there actually is a shootline. Originally i had TryFindCEShootLineFromTo disabled completely but then pawns would sometimes continuously fire at pawns that were partially concealed behind embrasures and as such waste all their ammo. Basically we need the cover and height checks CE does but they are calculated far too frequently. looking at the code a lot of these calls could be cached with a simple dictionary as well. I actually do this in my local version to squeeze out as much performance as possible.

pogoman commented 1 year ago

A great way to test this is spawn a setup like so: image And put some logging inside the CE Shootline function. Its insane how many times it gets called per second for 2 pawns 10 cells from each other. Now it doesnt matter so much in vanilla rimworld where the code being called for LoS is super basic, but for CE doing all the adjustshotheight calcs, additional z axis vectoring cover etc etc its a huge performance hit. So short of a rewrite of CE code caching is a quick fix here. If the caster and target havent changed cell, lean or stance position then you can create a composite key of this info and cache the result of the shootline check, saving yourself literally millions of LoS calls in any given raid

pogoman commented 1 year ago

Closing this issue as the circumstances that cause it are unique to my mod and CE. For academic purposes the root cause was my mod lets every raid breach at high range and when breaching units look for a firing position safeforrangedcast calls evaluatecell a number of times that exponentially increases with range from the target, and when the avoid grid is enabled and the player has enough turrets covering all viable firing positions more cells need to be evaluated which means shootline is called exponentially more times as well.

AluminumAlman commented 1 year ago

We're reopening this because it's responsible for lag spikes during raids: image JobGiver_AIFightEnemies calls TryFindShootingPosition, which calls CastPositionFinder.TryFindCastPosition, which calls CastPositionFinder.EvaluateCells, which calls verb.CanHitTargetFrom, which in CE calls TryFindCEShootLineFromTo, which calls CanHitFromCellIgnoringRange, which calls CanHitCellFromCellIgnoringRange. And CanHitCellFromCellIgnoringRange is responsible for the god-awful performance.