Stopping/Reducing microstutter on large func calls

[clicket]

Hello,

My addon attaches a frame to unitframes and displays auras on UNIT_AURA events per frame. The issue I'm having is that I currently look for frames from over 20+ different addons and because there's no way (that I know of) to know when a new frame has been added to the screen, I have to manually scan all frames on the screen to find attachment points for my units.

I do this by using some recursive scanning functionality, but when I run the scan (currently on GROUP_ROSTER_UPDATE) there is a micro stutter / slight freeze. Which, is fair given how hefty the function is.

My question is, is there a best practice for smoothing out large calls over a period of time to avoid this stutter without running into "script ran too long" issues? Currently use a coroutine, but that hasn't seemed to help in a meaningful way.

The repo for the code in question is here: https://github.com/clicketz/buff-ove...patibility.lua

But the relevant bits are here:

-- This is run out of combat to avoid "script ran too long" errors
-- Goal is to avoid massive coroutine garbage buildup blame
-- Unnecessary to functionality and a "needless" tax.
-- Can disable, but memory blame will be notable.
local collect = CreateFrame("Frame")
collect:SetScript("OnEvent", function(self)
    collectgarbage()
    self:UnregisterAllEvents()
end)
 
-- Main scan func
local function ScanFrames(depth, frame, ...)
    coroutine.yield()
    if not frame then return end
 
    if depth < maxDepth and frame.IsForbidden and not frame:IsForbidden() then
        local type = frame:GetObjectType()
        if type == "Frame" or type == "Button" then
            ScanFrames(depth + 1, frame:GetChildren())
 
            local name = frame:GetName()
            -- Make sure we only store unit frames
            local unit = SecureButton_GetUnit(frame)
 
            if name and unit and not BuffOverlay.frames[frame] then
                for enabledFrames, data in pairs(enabledFrameNames) do
                    if name:find(enabledFrames) then
                        BuffOverlay.frames[frame] = { unit = data.unit }
                        BuffOverlay:SetupContainer(frame)
                        break
                    end
                end
            end
        end
    end
    ScanFrames(depth, ...)
end
 
-- Used for unit validation per frame so we're not showing the wrong auras
function BuffOverlay:UpdateUnits()
    for frame, data in pairs(self.frames) do
        local unit = frame[data.unit] or SecureButton_GetUnit(frame)
 
        if unit and not data.blizz then
            self:AddUnitFrame(frame, unit)
        end
    end
    self:RefreshOverlays()
 
    if InCombatLockdown() then
        collect:RegisterEvent("PLAYER_REGEN_ENABLED")
    else
        collectgarbage()
    end
end
 
-- OnUpdate used for controlling coroutine
coFrame:Hide()
coFrame:SetScript("OnUpdate", function(self)
    local start = debugprofilestop()
 
    while debugprofilestop() - start < 15 and coroutine.status(co) ~= "dead" do
        coroutine.resume(co, 1, UIParent:GetChildren())
    end
 
    if coroutine.status(co) == "dead" then
        self:Hide()
        BuffOverlay:UpdateUnits()
    end
end)
 
-- Gets called every GROUP_ROSTER_UPDATE to maintain all visible frames from all addons that we're interested in
function BuffOverlay:GetAllFrames()
    if not coFrame:IsShown() then
        -- Timer is needed to account for addons that have a delay in creating their frames
        C_Timer.After(1, function()
            co = coroutine.create(ScanFrames)
            coFrame:Show()
        end)
    end
end

This does functionally work very well for my uses, but it would be nice to get rid of that stutter.

[Dridzt]

Calling collectgarbage() explicitly is a good way to make the problem worse

Other than that this reads like something that's "cool concept code" but shouldn't run in someone's addon folder.

There has to be a better way to anchor your frames to other frames that doesn't rely on recursively scanning on every group_roster_update which is quite spammy itself.

Edit: My suggestion would be to think about how to re-engineer it so it doesn't do that.

[clicket]

Originally Posted by Dridzt Calling collectgarbage() explicitly is a good way to make the problem worse

Yeah I fully agree. When I test the stutter I never run with it on but not calling it hasn't changed anything in my experience so I left it in for the post. Easy enough to disable, though. I currently use it to appease users who believe a high memory spike means poor performance or something's wrong, which has been complained about to me in the past.

Originally Posted by Dridzt Other than that this reads like something that's "cool concept code" but shouldn't run in someone's addon folder.

The recursive call is based on a library used by weakauras (LibGetFrame). I'm not sure it's as "cool concept but don't use" as you're implying. But, fair enough. I'd gladly stop using the scan if I could.

Originally Posted by Dridzt There has to be a better way to anchor your frames to other frames that doesn't rely on recursively scanning on every group_roster_update which is quite spammy itself.

I'm certainly open to suggestion, but I don't see another way to get frame information from N amount of addons without scanning for them. There are no events (that I know of) that will tell me when frames get added by addons (would love something like a COMPACT_UNIT_FRAME_CREATED event similar to NAME_PLATE_CREATED).

For Blizzard frames I just hook into CompactUnitFrame_UpdateAuras which gives me all the frame/unit information I need so there's no scan needed. But without each individual addon offering up that information in a way that everyone can access on event, I'm just not sure how else to go about it.

[Xrystal]

Are these other addons plugins to your addons or are you trying to access someone else's addon frame ?

The former can be *fixed* by adding some rules and steps those addons need to uphold and incorporate.

For example nUI allows plugins to be made that changes the skin and layout of the frames that nUI manages. nUI has a function called Registerxxxxxx where xxxx is a feature that nUI manages on the frames. It then talks to the other addon so that the other addon knows what to do or the other addon relays the data on registration that nUI then utilises when it calls its functions.

Whether this will work for your idea or not I don't know but it might provide you with an idea if it is along the right lines.

[clicket]

Originally Posted by Xrystal Are these other addons plugins to your addons or are you trying to access someone else's addon frame ?

Unfortunately, the latter. The addons are not plugins. I am grabbing the (raid) frame(s) that other addons create.

Would definitely love if all these addons just used an API from me instead

[Dridzt]

Originally Posted by clicket The recursive call is based on a library used by weakauras (LibGetFrame). I'm not sure it's as "cool concept but don't use" as you're implying. But, fair enough. I'd gladly stop using the scan if I could.

I haven't looked at WeakAuras code but I've used the addon and I'm willing to bet it uses that library to get a reference to a frame when you use the Frame Finder to add an aura to an actionbutton or whatever.

It would use that to get a reference when you set it up, I really doubt it's something called in an OnUpdate or some spammy event. They probably use it to find the frame store the reference to the aura and just do a lookup for trigger updates.


I'll think on it as to how you could do what you want to do a different way.

[clicket]

Originally Posted by Dridzt I haven't looked at WeakAuras code but I've used the addon and I'm willing to bet it uses that library to get a reference to a frame when you use the Frame Finder to add an aura to an actionbutton or whatever. It would use that to get a reference when you set it up, I really doubt it's something called in an OnUpdate or some spammy event. They probably use it to find the frame store the reference to the aura and just do a lookup for trigger updates.

I haven't looked too deeply at their code either. Thinking about it though, I *do* know you can dynamically attach auras to frames in their more recent versions. Perhaps I should look a bit closer...

Originally Posted by Dridzt I'll think on it as to how you could do what you want to do a different way.

I appreciate it! Not just to make my addon better, but also genuine curiosity. There doesn't seem (to me) to be too many avenues here besides trying to reduce the impact of the scan.

[Fizzlemizz]

All frames of the type you seem to be interested in are created in the one place. Someone who knows more about it than I might tell you it's a bad idea but wouldn't hooking CreateFrame and inspecting the Templates parameter do what you want rather than randomly searching through a bazillion frames every ...?

All that would be left to search are the frames created before your addon loaded like the default UI.

[clicket]

Originally Posted by Fizzlemizz All frames of the type you seem to be interested in are created in the one place. Someone who knows more about it than I might tell you it's a bad idea but wouldn't hooking CreateFrame and inspecting the Templates parameter do what you want rather than randomly searching through a bazillion frames every ...? All that would be left to search are the frames created before your addon loaded like the default UI.

That... is actually pretty hilarious. There are a couple (not insurmountable) hurdles, but overall it could work in theory.

I'm sure someone will chime in to tell us both how hilariously stupid that would be, but I like it!

[Fizzlemizz]

There's no reason I can see for CreateFrame to be "extra special function" but it's probably not what you want every addon doing and only with the absolute minimum required to find what you need.

But hookescurefunc is a hammer I was given so until proven otherwise...

[SDPhantom]

I'd be careful with hooking CreateFrame() as many areas of the DefaultUI generate dynamic frames, a lot of them are sensitive to taint and will cease functioning and/or spit out taint errors. Some examples are NamePlates and RaidFrames.

[clicket]

Originally Posted by SDPhantom I'd be careful with hooking CreateFrame() as many areas of the DefaultUI generate dynamic frames, a lot of them are sensitive to taint and will cease functioning and/or spit out taint errors. Some examples are NamePlates and RaidFrames.

Yeah, good call. Luckily I only care about addon frames for this, and pretty specific ones at that. I can pretty safely ignore forbidden frames.

I've been messing around a bit with this and I'm cautiously optimistic about it, but obviously if it just taints the entire UI I'd be very sad.

[SDPhantom]

The problem being, the only way to "secure hook" a function is with hooksecurefunc(), but it won't give you the return for the hooked function, just its args when called. The problem with hooking CreateFrame() in a more standard Lua way will taint any execution path calling it regardless of if you did anything to the created frame or not. The calling code will then likely either call protected functions itself or spread taint through storing its own variables that are then read by other functions that make protected function calls.

[clicket]

One of the args of CreateFrame is the frame name, so I just use that name to find the frame in _G and pass along the reference. I'm *hoping* there's nothing taint-worthy there, but I've been wrong about taint in the past.

hooksecurefunc("CreateFrame", function(frameType, name, _, template)
    if not addOnsExist then return end
 
    if frameType == "Button" and template then
        local frame = _G[name]
 
        if frame and not frame:IsForbidden() then
            if not name:match("BuffOverlayBar") then
                tempFramePool[frame] = true
            end
        end
    end
end)

It's working really well for my uses, although it does feel dirty.

[SDPhantom]

Frame name is a completely optional arg. If it was supplied, there would be no reason to hook CreateFrame() in the first place, you can just pull the frame from the global environment by that name anyway.

If you have named frames like this:

SomeAddOnUnitFrame1
SomeAddOnUnitFrame2
SomeAddOnUnitFrame3
SomeAddOnUnitFrame4

You can scan and access these quickly like this:

local i=1;
while true do-- Infinite loop, we'll break manually
    local button=_G["SomeAddOnUnitFrame"..i];
    if button then
--      Do something with the frame
 
        i=i+1;--    Don't forget to increment the counter
    else
        break;--    Manually break out of loop
    end
end

Note: The unique thing about a repeat ... until block is it can check the state of locals defined inside the loop.

[clicket]

Originally Posted by SDPhantom Frame name is a completely optional arg. If it was supplied, there would be no reason to hook CreateFrame() in the first place, you can just pull the frame from the global environment by that name anyway.

Yes, but for my purposes there will be no unnamed frames that I need so it works out. But I still need to hook it because otherwise I'm back in the situation of randomly scanning all the time to find frames that I don't know exist or not.

In general, there are also a lot of weird naming conventions for these addons. With grabbing frames indiscriminately I can just filter by pattern, but searching for a specific name with incrementing indices in multiple locations in the framename would be challenging to generalize for a loop like that across n addons.

For instance, here are three different patterns I currently use:

^ElvUF_Raid%d+Group%dUnitButton%d+$ -- ElvUI
^Vd%dH%d+$ -- VuhDo
^CellRaidFrameHeader%d+UnitButton%d+$ -- Cell

Each of these increments their digits differently (even sometimes differently from itself based on certain settings within the addon. Grid2 gets pretty wonky with that IIRC). How many times do I increment the first digit in each? The second? Does the third one ever increment or is it always "1"? Questions like these, since the answers change heavily between addons, make it exceedingly difficult to generalize, and also pretty heavily affect the runtime depending on the answers.

I could definitely just set up a separate scan function for every addon, and maybe that's smarter, but it seems less elegant to me. I also suspect there would be quite a few edge cases I miss with that approach since specificity matters. I'll think more on it.

Anyway, thanks for all the help!

[SDPhantom]

Any single addon isn't likely to change their naming scheme and it's doubtful people are going to use more than one UnitFrame addon as they are likely to conflict. You can check which is loaded and then scan for frames specific to that addon. Narrowing the scope of the scan in this way reduces the time needed for processing.

You can also cache what addons are loaded when you load and watch ADDON_LOADED if any are loaded after yours. Also, the code I posted, when an addon isn't there or frame hasn't been created yet, will immediately quit on the first check and not waste any further processing.

[clicket]

Originally Posted by SDPhantom Any single addon isn't likely to change their naming scheme

An example of this is when Grid2 is under default settings, all of its buttons are under Header1 something like Grid2Header1UnitButton1-40. When you enable a setting that forces all groups to show or some such setting, it separates everything into multiple headers, up to 9 I believe. I'd have to always look for header 1-9 and button 1-40, even though there will only ever be 40 buttons. (I could be misremembering, I don't have access to wow atm, but the point should be clear).

It's something like this, that gets caught by a pattern but not a specific search, that makes the specific search worse in my opinion. Because I'm not going to deep-dive into every setting of every single raid frame addon.

Originally Posted by SDPhantom You can check which is loaded and then scan for frames specific to that addon. Narrowing the scope of the scan in this way reduces the time needed for processing.

Yeah, agreed. I already do this.

[Fizzlemizz]

The problem is keeping up with new addons of the required type. The list of unit frame addons is not as small, ancient or unchaging as it once was. There's even a second DUF around these days .

[clicket]

Yeah, and with that in mind it's even more appealing to keep things as generalized as possible. I was pretty surprised when I started adding addons to support with just how many there are.