DynamicCam

DynamicCam

Addons
1M Downloads

Skyriding Performance Woes

emptyrivers opened this issue ยท 3 comments

commented

Hello, I've been having severe fps problems in War Within, and today I decided to make use of Perfy in the hopes that I would find a smoking gun. And I did!

Investigation

Here are some reports that Perfy generated for 30s of skyriding in loops around the populated bit of dornogal, near the upgrade npcs and timeways portal:
lowfps cpu
lowfps mem

Of Particular note to me was the DynamicCam:SkyridingOn() method, which seems to be running for an abnormally large amount of time.

Disabling DynamicCam resulted in a noticeably smoother subjective performance while skyriding, both in and out of Dornogal. For reference, here are some reports of a Perfy run I made, with DynamicCam disabled:
without_dynamiccam cpu
without_dynamiccam mem

Analysis

Reading the source code, I can see how this happens: the builtin situation controls for situations 103 Mounted (only flying-mount + airborne + Skyriding), 104 Mounted (only flying-mount + Skyriding), 106 Mounted (only airborne + Skyriding), and 107 Mounted (only Skyriding) (of which I had 106 & 107 enabled in this perf run) all invoke DynamicCam:SkyRidingOn() on PLAYER_MOUNT_DISPLAY_CHANGED (unlikely to be problematic) and UNIT_AURA (extremely spammy in heavily populated areas). And DynamicCam:SkyRidingOn() itself is a potentially quite expensive function to run:

DynamicCam/SituationManager.lua

Lines 989 to 1016 in 6c26720

function DynamicCam:SkyridingOn()
if self.lastActiveMount then
local _, _, _, isActive, _, _, _, _, _, _, _, _, isSteadyFlight = C_MountJournal.GetMountInfoByID(self.lastActiveMount)
if isActive and isSteadyFlight then
return false
end
end
for _, v in pairs (C_MountJournal.GetMountIDs()) do
local _, _, _, isActive, _, _, _, _, _, _, _, _, isSteadyFlight = C_MountJournal.GetMountInfoByID(v)
if isActive then
self.lastActiveMount = v
if isSteadyFlight then
return false
end
end
end
for i = 1, 40 do
local aura = C_UnitAuras.GetBuffDataByIndex("player", i)
if aura and aura.spellId == 404464 then return true end
if aura and aura.spellId == 404468 then return false end
end
-- If you have never switched, you have neither buff and Skyriding it the default.
return true
end

Contrast this with how WeakAuras detects whether the player is "skyriding", which we've used for 10 months now with no complaints from our users:

Private.IsDragonriding = function ()
  return UnitPowerBarID("player") == 631
end

(the relevant events are UNIT_POWER_BAR_SHOW/UNIT_POWER_BAR_HIDE/PLAYER_ENTERING_WORLD)

I believe DynamicCam could benefit a lot from adopting this method of detecting if the player is skyriding.

Experiments

Injecting the line do return UnitPowerBarID("player") == 631 end at the top of DynamicCam:SkyridingOn() produced the following Perfy report (DynamicCam is on the left again, note its vastly reduced CPU time):
with_dcam_fix cpu
with_dcam_fix mem
Note the similarity to the above run without DynamicCam enabled.

Editing the situation controls for:

  • Mounted (only Skyriding) to use events UNIT_POWER_BAR_SHOW, UNIT_POWER_BAR_HIDE and condition return UnitPowerBarID("player") == 631
  • Mounted (only airborne + SkyRiding) to use events PLAYER_IS_GLIDING_CHANGED, UNIT_POWER_BAR_SHOW, UNIT_POWER_BAR_HIDE and condition return IsFlying() and UnitPowerBarID("player") == 631

produced the following Perfy run:
custom_situation_controls cpu
custom_situation_controls mem

Subjectively, both changes were a marked improvement to flying around in dornogal, and outside dornogal, flying became buttery smooth. Given the crosstalk between SkyridingOn and CurrentMountCanFly, I'm not quite sure if the first change would break anything, so I decided to write up all my findings instead of simply doing a drive-by pull request that might not be completely sound.

commented

Great job! Thanks for this elaborate analysis. The skyriding stuff has been a very recent addition to the DynamicCam code. I will definitely check this out and resolve these issues with the next release!

commented

My SkyridingOn() function was in fact bugged such that it had to always traverse the entire mount journal. I fixed this and some further slight performance improvements with release 2.9.0.

I really want to try prefy myself some time, but I just did not have time tonight. Maybe you wanna profile DynamicCam again and let me know the results?

Thanks again!

commented

2.9.0 had a nasty bug that prevented situation events from being triggered. Should you run another test, please use 2.9.1 or above, please. :-)