2.1 and SetHandler

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

[merlight]

Originally Posted by Wandamey i mean, the handler has an identifier : control/"OnWhatever", i thought what you store in it would replace the old one

Yes, of course. The problem is probably not SetHandler per se. It's the fact that creating a closure requires a dynamic allocation (I don't even know whether just one, or one for each variable; depends on how much Lua optimizes that). Remember that everything you allocate must eventually be collected after you discard it. Too many closures might be thrashing the garbage collector.

Originally Posted by Wandamey 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

Local variables may only exist in registers (i.e. cost virtually nothing) as long as they're not used in any closure. Once you create a closure, they HAVE to be moved to dynamically allocated memory, because the closure typically outlives the block it has been created in.

Originally Posted by merlight Local variables may only exist in registers (i.e. cost virtually nothing) as long as they're not used in any closure. Once you create a closure, they HAVE to be moved to dynamically allocated memory, because the closure typically outlives the block it has been created in.

whow thanks.



as soon as my head get rif of this picture, it should be way more clear.

[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.

shouldn't buttonId be a member of self at some point?

(haven't looked at the whole code yet and i don't know when or how you declare it, but thats why i suggested to define your tooltip text outside btw)

[Baertram]

Defining it outside won't work as the settings could be changed after the handler was set.
And buttonId is a parameter of the calling function AddOrChangeFilterButton. So I guess, and that is what I asked :-), yes: it should be connected to the button control too.

Originally Posted by Baertram Defining it outside won't work as the settings could be changed after the handler was set. And buttonId is a parameter of the calling function AddOrChangeFilterButton. So I guess, and that is what I asked :-), yes: it should be connected to the button control too.

hhmmm, when you enter the mouse, it'll use self.tooltiptext aka button.tooltiptext which calls the function, so it should be updated.

i should look at your code first, just don't listen to me. am just writing out loud.

ok, i looked at it... i saw keanu reeves and a colored blob at first but still 2 solutions maybe?

either just redefine
button.buttonId = buttonId
(same with the panelId i suppose)

or button.tooltipText = function(blabla) <-- put this outside of your check : if (checkIfButtonexists == nil) , so it's updated when it needs to be.
And, the most important here : wait for another advice than mine

[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.

[sirinsidiator]

Originally Posted by merlight 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.

I might have missed GetGameTimeMilliseconds. The other time functions that I tested returned the same value every time.
What I did was something like this:

local GetTime = WhateverTimeFunction
function SomeFunction()
  local start = GetTime()
  -- do something that makes the game freeze for a second
  df("This took %d", GetTime() - start)
end

That's pretty tedious to add to every function though.

Best case we would get a value in UserSettings.txt which enables a profiling event that returns data for each frame.
EVENT_FRAME_EXECUTION_TIMES ( integer frameTime, integer cpuPercent, integer memoryAllocation, object callstack, object addonData )
Callstack would contain all calls that happened during the frame in tree form:

callstack = {
  [1] = {
    functionName = "MyFunction",
    callTime = 123456789, -- nanoseconds?
    file = "MyAddon.lua",
    line = "123"
    callstack = {
      [1] = { ... }, -- same as before
      [2] = { ... }
    },
    [2] = { ... },
}

AddonData shows the cpuPercent and memoryAllocation for individual addons.

addonData = {
  [1] = {
    addonName = "MyAddon",
    cpuPercent = 12, -- %
    memoryAllocation = 123456 -- Bytes
  }
}

[Fyrakin]

Profiling event would certainly help to narrow down the most problematic pieces.

[Lodur]

I just found in MailLooter I was calling SetHandler 4 times in my ZO_ScrollList row setup function. This can be called a lot if your scrolling through the list...

Since ZO_ScrollList uses a row template - the only way to fix this is in XML.

Just a heads up for anyone else doing what I did with a ZO_ScrollList.