Help to track down memory leak

[QuadroTony]

it connected with Shissu guild tools addon
i think we together can help to fix it faster
afaik Shissu tried to fix it in last updates but without luck

www.youtube.com/watch?v=2cANGJ1RfoQ


P.S. also i kicked for spamming when promoting/demoting too fast, like 2 per second
its weird because its not fast enough to be kicked

probably because of SGT too

[merlight]

Looked at the code, and found two points that might be a start to identifying why it happens.

1. SGT alters the scroll list in a very evil and inefficient way.

Hooking ZO_ScrollList_UpdateScroll and other generic functions is not nice. All hooks referecing SGT_Roster.ColumnUpdateHandler should be removed and replaced with just this one:

local orgSetupGuildMember = GUILD_ROSTER_MANAGER.SetupGuildMember
function GUILD_ROSTER_MANAGER:SetupGuildMember(control, data)
    orgSetupGuildMember(self, control, data)
    SGT_Roster.NewColumnAddRow(control, data)
    -- it currently doesn't use the second arg, but could, for convenience
end

It could be further optimized by moving sub-control creation to row-control creation callback, but that'd be more work for a negligible effect, so not really that necessary.


2. SGT_Roster.NewColumnAddRow could use a little optimization.

In my opinion it shouldn't construct tooltip strings at all. Their construction should be deferred to the moment right before tooltip display, i.e. OnMouseEnter.

Btw, I'm just guessing, but perhaps the tooltip creation is what eats the memory. There's a loop over MemberChars, with character names being appended to the tooltip one by one, each after the first doing 2 string concats.

ShissuGT.Lib.GetCharInfoIcon does 0 to 3 concats. It should be changed to string.format("|t28:28:%s|t%s", icon, text) -- that will remove the 2 intermediate string allocations.

SGT_Roster.NewCharName consequently does 1 to 7 concats. With fixed GetCharInfoIcon it will be 1 to 3.

So for 8 characters, there will be (1 to 7) + 7 * (3 to 9) concatenations. Due to all the above points combined, this happens every time you scroll the list for all people you currently see (roughly 20). That's 400-1200 string concats. I don't know when Lua discards unused strings, I hope it does at some point, but this looks like it's allocating too many too fast.

Refactored tooltip construction

local function buildTooltip(MemberChars)
    local lines = {}
    for charName, charInfo in pairs(MemberChars) do
        lines[#lines + 1] = SGT_Roster.NewCharName(charName, charInfo)
    end
    table.sort(lines) -- making the order consistent can't hurt ;)
    return table.concat(lines, "\n")
end
 
charNameControl.tooltip = nil
 
charNameControl:SetHandler("OnMouseEnter", function (self)
    if self.tooltip == nil then
        local MemberChars = SGT_Roster.MemberChars[displayName]
        if MemberChars then
            self.tooltip = buildTooltip(MemberChars)
        else
            self.tooltip = false
        end
    end
    if self.tooltip then
        ZO_Tooltips_ShowTextTooltip(self, TOP, self.tooltip)
    end
end)
 
charNameControl:SetHandler("OnMouseEnter", function (self)
    ZO_Tooltips_HideTextTooltip()
end)

[votan]

What about using enclosure variable charNameControl instead of self. Shouldn't "self" be the same reference as "charNameControl" and does it matter at all?
I'm still confused about this enclosure magic and it's implicit memory usage.

/edit: You understood me right at first time. I just was not sure, if its really matter or the enclosure will exist anyway, because it is a nested function.

[merlight]

Originally Posted by votan What about using enclosure variable charNameControl instead of self. Shouldn't "self" be the same reference as "charNameControl" and does it matter at all? I'm still confused about this enclosure magic and it's implicit memory usage.

You're right, I should've used "self" in the handler; was copying the pieces from outer scope and forgot to replace them. Code edited.

Edit: oh you meant the other way around? I suppose local variable access (function arguments are local variables, too) is simpler than closure variable access. Specifically, using self, charNameControl doesn't need to be in the closure at all.

[circonian]

I agree with what merlight said. The biggest problem looks like those functions, the prehook on ZO_ScrollList_MouseEnter (fires no matter what scroll list your mouse enters), and mainly the prehook on ZO_ScrollList_UpdateScroll. Scrolling a single line caused that update preHook to fire 26 times. Which in turn loops through all 19 active controls for each of the 26 fires which means its processing all of that information for each line you scroll in the scrollList 19*26 = 494 times (minimum, thats not even counting mouseEnter fires as the list scrolls).

[Shissu]

in a newer version the problem is solved....

i dont use then more the activecontrols, i change the roster direct - but first update 7 fix....