2.1 and SetHandler

[ZOS_ChipHilseberg]

We've been profiling some UI code since 2.1 went live and we've noticed some spikes with SetHandler on controls. Especially when the code is frequently making a new anonymous closure and passing that to SetHandler. In a lot of cases this can be avoided during play time by setting the handler on the control in the XML or on a template (virtual = true) in the XML so that cost is paid during the load. The handler closure should also only be made once instead of every time SetHandler is called, by storing the closure in a local or on a table.

probably a stupid question but i suppose someone has to ask it...

what is a closure?

an example would be nice too.

[Harven]

Hey,
Wandamey, quick google search and here it is: http://www.lua.org/pil/6.1.html

...

Is there a difference between setting the Handler in a xml or setting on load with a control creation in lua?

now that i see what a closure is (in the great lines) i'd still like an example... i mean related to what Chip said.

[merlight]

I'll try to provide a concrete example from some add-on, I'll probably use Fyrakin's MiniMap because it seems to have been hit hard by this.

Originally Posted by merlight I'll try to provide a concrete example from some add-on, I'll probably use Fyrakin's MiniMap because it seems to have been hit hard by this.

I hope you can make it 3 lines with pictures


the big picture i see here is : store your functions first. but how do i pass a variable to it without using an Anonymous func again to call it? -- maybe the table?

[merlight]

Ok found something that might illustrate the issue and possible solution. From MiniMap.lua function AddLocation:

local pinTooltipInfo = ZO_MapPin.TOOLTIP_CREATORS[MAP_PIN_TYPE_LOCATION]
locationPin:SetHandler("OnMouseEnter", function(pin)
    FyrMM.SetTargetScale(pin, 1.3)
    if not FyrMM.SV.PinTooltips then return end
    if pinTooltipInfo.tooltip then
        InitializeTooltip(ZO_MapLocationTooltip, Fyr_MM, TOPLEFT, 0, 0)
        pinTooltipInfo.creator(pin.m_Pin)
        IsCurrentLocation(pin)
    end
end)

The anonymous function uses pinTooltipInfo from outer scope, thus each time this runs, a new closure holding that local variable must be created.

Possible solution: define the handler function outside AddLocation, and change it to obtain pinTooltipInfo in a different way -- either make it a member of an argument it gets, like this:

locationPin.pinTooltipInfo = ZO_MapPin.TOOLTIP_CREATORS[MAP_PIN_TYPE_LOCATION]
locationPin:SetHandler("OnMouseEnter", locationPin_OnMouseEnter)

or in this case it can easily be obtained from ZO_MapPin in the handler itself:

local function locationPin_OnMouseEnter(pin)
    FyrMM.SetTargetScale(pin, 1.3)
    if not FyrMM.SV.PinTooltips then return end
    local pinTooltipInfo = ZO_MapPin.TOOLTIP_CREATORS[pin.m_PinType]
    if pinTooltipInfo.tooltip then
        InitializeTooltip(ZO_MapLocationTooltip, Fyr_MM, TOPLEFT, 0, 0)
        pinTooltipInfo.creator(pin.m_Pin)
        IsCurrentLocation(pin)
    end
end)

[Lodur]

Originally Posted by Wandamey probably a stupid question but i suppose someone has to ask it... what is a closure? an example would be nice too.

A closure is a combination of context and code...

Originally Posted by Fyrakin What about self.your variable?

thanks, i figured it out with votan example, and everything is uploaded.
I was afraid i wouldn't use properly the "self", but i would have used an addon variable to temporary store it if it hadn't worked.
surprisingly, it went so flawlessly that I had to recheck 3 times that i didn't edit in the pts folder instead of the live one

Originally Posted by Lodur A closure is a combination of context and code...

that's what i understood from harven's link, but I still dont understand why it doesn't simply replace the previous one when the context is the same. Not that it prevents me from doing what must be done, but it is still cloudy unclear.

[merlight]

Originally Posted by Wandamey ... I still dont understand why it doesn't simply replace the previous one when the context is the same.

Think about this for a while. The context is not the same on each run, that's the whole point of creating a closure. Outer local variables are different, completely separate from the previous run's local variables.

it's a bit hard to tell i I don't know how it is stored and processed in the first place.

i mean, the handler has an identifier : control/"OnWhatever", i thought what you store in it would replace the old one, and eventually replace the previous results with the new ones when ran again, why would the memory keep the old runs ad vitam aeteram?


edit.. well ok, different variables... it doesn't explain why rewriting a stored procedure like in the example you gave me or like votan asked is that bad. (i see it can be better by not running unecessary calls, but it aint the end of the world either, i mean, ZOS_DanB even recommanded to do an update all session long when DerBombo asked for an EVENT that triggers at the end of the session...)


re edit : to understand why i'm a bit puzzled, maybe i should add that until now I thought that the interest of having local vars was that (i believed) they were trashed after the function was run

[Baertram]

To see if I understood everything correct about the performance stuff I'd like to show you an example and ask, how I need to change it (including ideas).

Current code:

button:SetHandler("OnMouseEnter", function(self)
--tooltipText is a local defined variable, outside of the closure, but inside the same function which sets the handler                  
tooltipText = outputFilterState(false, settingsVars.settings.splitFilters, p_FilterPanelId, buttonId, mappingVars.settingsFilterStateToText[tostring(getSettingsIsFilterOn(buttonId, p_FilterPanelId))])
                    if tooltipText ~= "" then
                        local showToolTip = true
                        --Don't show a tooltip if the context menu for gear sets is shown at the filter button
                        if contextMenu.GearSetFilter[locVars.gFilterWhere] ~= nil then
                            showToolTip = contextMenu.GearSetFilter[locVars.gFilterWhere]:IsHidden()
                        end
                        --Don't show a tooltip if the context menu for research, deconstruction & improvement is shown at the filter button
                        if showToolTip and contextMenu.ResDecImpFilter[locVars.gFilterWhere] ~= nil then
                            showToolTip = contextMenu.ResDecImpFilter[locVars.gFilterWhere]:IsHidden()
                        end
                        if showToolTip and contextMenu.SellGuildIntFilter[locVars.gFilterWhere] ~= nil then
                            showToolTip = contextMenu.SellGuildIntFilter[locVars.gFilterWhere]:IsHidden()
                        end
                        if showToolTip then
                            ZO_Tooltips_ShowTextTooltip(button, BOTTOM, tooltipText)
                        end
                    end
                end)

Optimized code:

button.tooltipText = tooltipText
button:SetHandler("OnMouseEnter", function(self)
    button.tooltipText = outputFilterState(false, settingsVars.settings.splitFilters, p_FilterPanelId, buttonId, mappingVars.settingsFilterStateToText[tostring(getSettingsIsFilterOn(buttonId, p_FilterPanelId))])
    if self.tooltipText ~= "" then
        local showToolTip = true
        --Don't show a tooltip if the context menu for gear sets is shown at the filter button
        if contextMenu.GearSetFilter[locVars.gFilterWhere] ~= nil then
            showToolTip = contextMenu.GearSetFilter[locVars.gFilterWhere]:IsHidden()
        end
        --Don't show a tooltip if the context menu for research, deconstruction & improvement is shown at the filter button
        if showToolTip and contextMenu.ResDecImpFilter[locVars.gFilterWhere] ~= nil then
            showToolTip = contextMenu.ResDecImpFilter[locVars.gFilterWhere]:IsHidden()
        end
                    if showToolTip and contextMenu.SellGuildIntFilter[locVars.gFilterWhere] ~= nil then
                        showToolTip = contextMenu.SellGuildIntFilter[locVars.gFilterWhere]:IsHidden()
                    end
        if showToolTip then
            ZO_Tooltips_ShowTextTooltip(self, BOTTOM, self.tooltipText)
        end
    end
end)

I changed the button variable inside the anonymous SetHandler function to use the "self".
And I changed the local tooltipText variable usage inside the anonymous SetHandler function to use the self.tooltipText variable now, which was passed to the button control outside the closure.

But what about all the other variables used inside the anonymous SetHandler function, like "buttonId" (which is a parameter of the calling function, which is setting the handler), "settingsVars.settings.splitFilters" (which is an addon wide known local variable set as the settings in LAM 2.0 panel get changed), "p_FilterPanelId" (which is a parameter of the calling function too)?
Do I need to pass them to the button control too and use the self.variableName inside the anonymouse function then?

Originally Posted by Baertram snip

i think you should define : in the first example
button.tooltiptext = tooltiptext

-- Edit : and define it here, not in the function btw with

button.tooltipText = outputFilterState(false, settingsVars.settings.splitFilters, p_FilterPanelId, buttonId, mappingVars.settingsFilterStateToText[tostring(getSettingsIsFilterOn(buttonId, p_FilterPanelId))])

elsewhere you define this

local function MyFunction(self)

 local  tooltiptext = self.tooltiptext   -- just to keep the same variable and not mix everything up if your function is long like my arm - self here is button, cause it's called from the button

   blabla all your code from the anonymous function blabla

end

then for the Handler :

button:SetHandler("OnMouseEnter", MyFunction)

now lets the teachers look at the homework

[merlight]

Originally Posted by Baertram I changed the button variable inside the anonymous SetHandler function to use the "self".

Yes, that's good.

Originally Posted by Baertram And I changed the local tooltipText variable usage inside the anonymous SetHandler function to use the self.tooltipText variable now, which was passed to the button control outside the closure.

I don't see why it can't be local inside the anonymous function, you're assigning it from outputFilterState() right there. Edit: I guess you want to call outputFilterState inside the handler, not before it's set, because settings might change in-between.

outputFilterState, buttonId, settingsVars... everything that's local above the anonymous function definition goes into the closure's context.

I think you don't need to worry about that if you're only doing the whole thing once per user action. I'm saying this because that's what I do. For example here:
https://github.com/merlight/eso-merT...window.lua#L52. This function is called once after window creation. There's one closure for savePos and one for resizeStart, and they're used for handlers on lines 58-60. But each time resizeStart is called, it creates a new closure for OnUpdate handler. Does it bother me? I could put self or panel inside control and define that handler outside. But resizeStart is only called once per click on control border, that's not enough to force me to make it a little bit more convoluted.

[Baertram]

Thanks for the hints. I'll try to change it to local functions instead of the anonymous then too.

Ok maybe it is easier to understand where the current variables are used, and how they are used, if you take a look at the current version of FCOItemSaver and look inside the function "AddOrChangeFilterButton(...)" in source code line 6300.

The SetHandler call I described above is in line 6336.

The tooltipText could be inside the closure too, yes. I don't know why I defined it outside in the past as it will be overwritten by the outputFilterState(...) function again. I guess I had some reason and need to change and test to find it again (or drop it ). Thanks.
But you're right: I'm only setting the handler, and updating the tooltip text, if the user decided to use the tooltip for that button.
So if he changes the settings and moves the mouse over the existing button the handler needs to be updated to hide the tooltip instead. I guess I need to NIL the handler then too before changing it.

This example here, function AddOrChangeFilterButton(...), is called everytime you open the crafting station, mail panel, player trade panel, guild bank, bank to update the inventory filter buttons. So it is not used constantly but after a user action, right.

[sirinsidiator]

Originally Posted by ZOS_ChipHilseberg We've been profiling some UI code since 2.1 went live and we've noticed some spikes with SetHandler on controls. Especially when the code is frequently making a new anonymous closure and passing that to SetHandler. In a lot of cases this can be avoided during play time by setting the handler on the control in the XML or on a template (virtual = true) in the XML so that cost is paid during the load. The handler closure should also only be made once instead of every time SetHandler is called, by storing the closure in a local or on a table.

Any chances you could add some functions for profiling to the API?
It would be enough if we had a function that gave us an updated time inside a function in order to allow us to write our own tools.
As far as I have seen all time functions only update between execution, so if I called GetGameTimeMilliseconds() twice with some heavy processing in between I would get the same value from both calls.

[Fyrakin]

Originally Posted by sirinsidiator Any chances you could add some functions for profiling to the API? It would be enough if we had a function that gave us an updated time inside a function in order to allow us to write our own tools. As far as I have seen all time functions only update between execution, so if I called GetGameTimeMilliseconds() twice with some heavy processing in between I would get the same value from both calls.

For this purpose you can easily write a callback, but I agree we could use some profiling means, IE memory usage by add-on, cpu costs by add-on etc.

[sirinsidiator]

I could, but who knows what happens before that callback gets called. It would make the result pretty much useless IMO.

[Fyrakin]

I would make a callback logger and call it whenever I want to log something. Process with many forks can do the callback with a parameter where it can send some essential info. If something and somewhere happens logger might have a clue if it get a sensible info through parameter.

[merlight]

Originally Posted by sirinsidiator Any chances you could add some functions for profiling to the API? It would be enough if we had a function that gave us an updated time inside a function in order to allow us to write our own tools. As far as I have seen all time functions only update between execution, so if I called GetGameTimeMilliseconds() twice with some heavy processing in between I would get the same value from both calls.

GetFrameTimeMilliseconds() returns the same value during a single UI frame, yes.

GetGameTimeMilliseconds() returns actual time since the game was started, so it basically works for measuring time spent within a function, but the results are not very consistent. They depend on thread scheduling (how much CPU time the Lua VM is given), how it struggles with memory allocations etc. If you want to evaluate whether func1 is more efficient than func2, you'll have to call them hundreds of thousands of times to get some usable averages.