Thread Tools Display Modes
08-23-13, 11:15 AM   #1
Malsomnus
A Cobalt Mageweaver
AddOn Author - Click to view addons
Join Date: Apr 2013
Posts: 203
About add-ons optimization

Hey, just thought I'd ask for some good tips for optimizing my add-ons. Any really obvious things I should do/ tools I should use?

So far what I've been doing is focusing on functions that run frequently (OnUpdate, UNIT_AURA, log events) and removing all calls to global functions/ variables, plus making sure no variables are created in that scope. The only tool I have for it is WowGlobalFinder, and, err, the game's default display of the 3 add-ons that take up the most memory.
What other techniques and add-ons should I know about? I can only assume there are a bunch of tips and tricks for working with strings and tables that I'm not aware of...
__________________
SanityCheck - If you've ever said the words "Sorry, I forgot" then you need this add-on.

Remember, every time you post a comment on an add-on, a kitten gets its wings!

Last edited by Malsomnus : 08-23-13 at 11:21 AM.
  Reply With Quote
08-23-13, 03:10 PM   #2
Phanx
Cat.
 
Phanx's Avatar
AddOn Author - Click to view addons
Join Date: Mar 2006
Posts: 5,617
In descending order, I'd say these should be your areas of focus:

1. Make sure you are not leaking any globals, and reconsider the necessity of any intentional globals, including the names of frames and other objects. Your main addon object and/or main display frame are good candidates for global names; other objects generally don't need names unless you're using Blizzard templates that require them.

2. Upvalue any globals you're reading in frequently called functions, like OnUpdate, CLEU, UNIT_AURA, etc. Don't bother upvaluing globals you only read infrequently, such as in response to the user changing an option, the player leaving combat, etc.

3. Avoid using OnUpdate scripts for anything other than updating a visual display, such as a statusbar, and move as much of that work as you can to other places, such as event handlers. For example, if you're showing a timer bar for a buff on the player, don't call UnitAura each time your OnUpdate script runs -- store the info you need in variables, and update those variables when UNIT_AURA fires for the player unit instead. Depending on what you're updating, you may be able to offload some/all of the work into C code by using animations.

4. Avoid calling functions when you can, as it's really slow. Some common examples of unnecessary function calls include:

(a) using local x = select(2, ...) instead of local _, x = ...
(b) using string.format("raid%d", i) instead of "raid"..i
(c) using for i, v in ipairs(tbl) do instead of for i = 1, #tbl do local v = tbl[ i ]

5. Avoid creating new tables when you can reuse existing tables. In keeping with #4, don't use wipe unless you actually need to -- for example, if you have a table that's storing info about a buff obtained via UnitAura, you don't need to wipe it, since the keys don't change, and you just overwrite the values.

6. Keep variables limited to the narrowest scope necessary. For example, if a variable is only used inside a loop, define it locally inside the loop, not outside.

7. Call string functions directly, eg. format("banana", "%l", strupper, 1) instead of of ("banana"):format("%l", strupper, 1) -- both incur the cost of a function call, but the second also incurs the costs of getting a metatable and performing a table lookup. If you upvalue format then the direct-call method is enormously faster, but even as a global lookup it's still slightly faster. The only time I'd recommend using the meta-method is when you're chaining multiple string operations; in that case, it's still the slower way to do it, but the increased readability of your code is usually worth it.

If you want more specifics, post your code.
__________________
Retired author of too many addons.
Message me if you're interested in taking over one of my addons.
Don’t message me about addon bugs or programming questions.

Last edited by Phanx : 08-23-13 at 03:14 PM.
  Reply With Quote
08-23-13, 04:13 PM   #3
Malsomnus
A Cobalt Mageweaver
AddOn Author - Click to view addons
Join Date: Apr 2013
Posts: 203
No, I don't think you want me to post the thousands of lines of all my add-ons

Good tips, I especially like 5 (because I haven't thought of it at all).
However, 4 surprised me a bit. When I was doing my testing, defining a single number variable in OnUpdate had a very visible effect on my performance; calling a function (or twenty) didn't seem to have such an effect.
__________________
SanityCheck - If you've ever said the words "Sorry, I forgot" then you need this add-on.

Remember, every time you post a comment on an add-on, a kitten gets its wings!
  Reply With Quote
08-23-13, 05:07 PM   #4
Phanx
Cat.
 
Phanx's Avatar
AddOn Author - Click to view addons
Join Date: Mar 2006
Posts: 5,617
Static memory usage is almost completely irrelevant, and I have nothing but contempt for whichever Blizzard developer thought it would be a good idea to put "how much memory your addons are using" into the default UI. Most users (and most addon developers) do not understand what those numbers mean, and having them displayed just leads to annoyed developers having to explain over and over that "lower memory usage" is not intrinsically good or a worthwhile goal.

The metrics you should be caring about -- the ones that actually matter -- are increasing memory usage (how quickly your addon's memory usage is growing) and CPU time. Those are the things that actually affect framerates. I use OptionHouse to measure them, but there are plenty of other options, including Broker plugins. Make sure you disable CPU profiling when you're done, as leaving it enabled will slow down everything across the board.

This thread on WowAce from last year includes some benchmark tests I did showing the differences in speed (CPU time) for global lookups vs. upvalues, and different styles of calling string methods:

http://forums.wowace.com/showthread....000#post322000

Also, you should just attach your files, or post a link to your addon download. It doesn't really matter whether it's 100 lines or 100,000 lines; any bad coding habits will be just as easy to scan through and see regardless of the size, and I have a lot of free time at work.
__________________
Retired author of too many addons.
Message me if you're interested in taking over one of my addons.
Don’t message me about addon bugs or programming questions.
  Reply With Quote
08-23-13, 05:20 PM   #5
Malsomnus
A Cobalt Mageweaver
AddOn Author - Click to view addons
Join Date: Apr 2013
Posts: 203
Don't worry, I get that static memory bit. What I meant was, that display helped me see how SanityCheck was taking up rapidly increasing amounts of memory.
I wouldn't go as far as to flame the very idea of that display... my machine, for one, is old enough for me to care if I have several heavy add-ons loaded, even if they don't use much CPU. I doubt it was intended for this purpose; Blizzard, or any other sane organization, would not put that kind of tool on the main display of their game.

Also, just to make sure we're on the same page: when you say "upvalue" you mean this sort of assignment, right?
Lua Code:
  1. local _G = _G

Anyway, I will definitely get that OptionHouse and see about that old thread, thanks.
And if you're truly bored enough, please have a look at this and this and maybe this. Do not under any circumstances look at the code of MooTrack, it was my first add-on over 50 lines and the code sucks immensely
__________________
SanityCheck - If you've ever said the words "Sorry, I forgot" then you need this add-on.

Remember, every time you post a comment on an add-on, a kitten gets its wings!

Last edited by Malsomnus : 08-23-13 at 05:29 PM.
  Reply With Quote
08-23-13, 08:47 PM   #6
Rainrider
A Firelord
AddOn Author - Click to view addons
Join Date: Nov 2008
Posts: 454
Originally Posted by Phanx View Post
6. Keep variables limited to the narrowest scope necessary. For example, if a variable is only used inside a loop, define it locally inside the loop, not outside.
I have the following code:
lua Code:
  1. local AddComboPointsBar = function(self, width, height, spacing)
  2.     local comboPoints = {}
  3.     local maxCPoints = MAX_COMBO_POINTS
  4.  
  5.     for i = 1, maxCPoints do
  6.         local cPoint = self.Overlay:CreateTexture("oUF_Rain_ComboPoint_"..i, "OVERLAY")
  7.         cPoint:SetSize((width - maxCPoints * spacing - spacing) / maxCPoints, height)
  8.         cPoint:SetPoint("BOTTOMLEFT", self.Overlay, (i - 1) * cPoint:GetWidth() + i * spacing, 1)
  9.         cPoint:SetTexture(unpack(ns.colors.cpoints[i]))
  10.         comboPoints[i] = cPoint
  11.     end
  12.  
  13.     self.CPoints = comboPoints
  14. end

If we would ignore the fact that I need maxCPoints as an upper bound for the loop for the sake of the example, wouldn't it be still better to define maxCPoints outside the for-loop, despite it is only needed inside? Wouldn't this spare me a global look-up for MAX_COMBO_POINTS and a garbage collection on every loop iteration? I don't know how GC works in loops, but if it collects on every iteration would this be cheaper than having to move one scope up to find maxCPoints?

The other question is whether it is better to use:
Code:
cPoint:SetTexture(ns.colors.cpoints[i][1], ns.colors.cpoints[i][2], ns.colors.cpoints[i][3])
instead of the function call to unpack? I mean is there something like "a function call equals this many table look-ups"?
  Reply With Quote
08-23-13, 09:39 PM   #7
SDPhantom
A Pyroguard Emberseer
 
SDPhantom's Avatar
AddOn Author - Click to view addons
Join Date: Jul 2006
Posts: 2,313
That's actually more specific of a situation, as your definition of maxCPoints is needed in all iterations in the for loop, it's best to keep it where it is instead of reinitializing the same value to the same local multiple times. This can impact performance to do so unnecessarily.

As a rule of thumb, if a variable is needed in all iterations of a loop instead of in each independent iteration, it's best to define it as an upvalue instead of inside the loop.



As far as using unpack() versus manually indexing tables, it depends on how you're indexing the table for each return and at some point, I would think unpack() would start to be more appealing, but no data exists to suggest at what point (if any) this will start to happen.

It can be guessed at that indexing the table will happen in C side when dealing with unpack(), but another question is how would it compare to manual indexing in a single assignment statement? To make the comparison fair, the manual indexing will need to be a single-dimensional table. For example, ns.colors.cpoints[i][1] will use 4 indexing operations for a single value when storing the specific table into a local, then indexing it in your function call will result in only 1 index operation per value.

For example:
Code:
local points=ns.colors.cpoints[i];
cPoint:SetTexture(points[1], points[2], points[3]);
__________________
WoWInterface AddOns
"All I want is a pretty girl, a decent meal, and the right to shoot lightning at fools."
-Anders (Dragon Age: Origins - Awakening)

Last edited by SDPhantom : 08-23-13 at 10:00 PM.
  Reply With Quote
08-24-13, 08:18 AM   #8
Rainrider
A Firelord
AddOn Author - Click to view addons
Join Date: Nov 2008
Posts: 454
What is the difference between "needed in all iterations" and "needed in each independent iteration"? Please ignore the use of maxCPoints as the upper bound of the loop as this would be an obvious reason to define it locally in a higher scope as I also need it in each iteration. I thought that the point here would be that I assign a global (MAX_COMBO_POINTS) to maxCPoints, so that I would have a local variable definition, a global look-up and a garbage collect(??) for each loop iteration, where the global look-up is the most expensive part. So would the global look-up be reason enough to define maxCPoints outside the loop despite it being used only in the loop?
  Reply With Quote
08-24-13, 09:34 AM   #9
Vrul
A Scalebane Royal Guard
 
Vrul's Avatar
AddOn Author - Click to view addons
Join Date: Nov 2007
Posts: 404
Originally Posted by Rainrider View Post
What is the difference between "needed in all iterations" and "needed in each independent iteration"? Please ignore the use of maxCPoints as the upper bound of the loop as this would be an obvious reason to define it locally in a higher scope as I also need it in each iteration. I thought that the point here would be that I assign a global (MAX_COMBO_POINTS) to maxCPoints, so that I would have a local variable definition, a global look-up and a garbage collect(??) for each loop iteration, where the global look-up is the most expensive part. So would the global look-up be reason enough to define maxCPoints outside the loop despite it being used only in the loop?
I think you misunderstood the message Phanx was trying to convey. She meant more along the lines of your cPoint definition inside the loop as being the correct way to do that as some people would define cPoint outside the loop and update it within the loop in the hopes of avoiding non-existent overhead from garbage collection.

A good rule of thumb is that the cost of defining a local vs indexing hits the break even point at 3 look-ups. So, if you need an indexed value 3+ times you are not hurting anything by creating a local reference and using it instead.

That said, you do not need MAX_COMBO_POINTS within the loop at all. The calculation you are doing is static for each iteration so it only needs to be done once prior to the loop. Doing that also lets you get rid of the cPoint:GetWidth() index/call as you will already have that value.
Code:
local AddComboPointsBar = function(self, width, height, spacing)
    local comboPoints, overlay, pointColors = {}, self.Overlay, ns.colors.cpoints
    width = (width - (MAX_COMBO_POINTS - 1) * spacing) / MAX_COMBO_POINTS
    spacing = width + spacing

    for i = 1, MAX_COMBO_POINTS do
        local cPoint = overlay:CreateTexture(nil, "OVERLAY")
        cPoint:SetSize(width, height)
        cPoint:SetPoint("BOTTOMLEFT", (i - 1) * spacing + 1, 1)
        local color = pointColors[i]
        cPoint:SetTexture(color[1], color[2], color[3], color[4])
        comboPoints[i] = cPoint
    end

    self.CPoints = comboPoints
end
Changes:
1. Moved the size calculations to outside the loop
2. Remove the name of the points to avoid clutter in _G
3. Changed the math a bit on sizing to make full use of the width passed
4. Used local references for indexes used in the loop (if you don't use the alpha color parameter remove the color[4] bit)

With that said, I can't imagine that function ever being called enough times to have ever warranted more that a quick pass for optimization. You only really need to scrutinize code for heavily called stuff like OnUpdate scripts and certain event handlers (CLEU, UNIT_AURA, etc...).
  Reply With Quote
08-24-13, 11:45 AM   #10
Rainrider
A Firelord
AddOn Author - Click to view addons
Join Date: Nov 2008
Posts: 454
You are totally right, Vrul, thank you for your proposal, I'm going to make the changes. The loop iterates only 5 times and the function is called only once for the target frame in my oUF layout, so yes, it doesn't need that much of optimization. The global names are so I could debug frames and textures positioning with /fstack (had hard time with those because of wrong parenting and overuse of SetFrameLevel and stuff). So yes, it is overall a bad example for the need of optimization, I just wanted to show some real code as it raised questions for me and I could apply the answers elsewhere.

So, assumed I cannot move the calculation involving MAX_COMBO_POINTS, it's better to define it outside the loop so that I could spare the global look-up on every loop iteration. If it would be a local function call, or a local table look-up or a literal, I shall declare the variable for this in the loop. Is this the right way to sum it up?
  Reply With Quote
08-24-13, 12:57 PM   #11
Vrul
A Scalebane Royal Guard
 
Vrul's Avatar
AddOn Author - Click to view addons
Join Date: Nov 2007
Posts: 404
Originally Posted by Rainrider View Post
So, assumed I cannot move the calculation involving MAX_COMBO_POINTS, it's better to define it outside the loop so that I could spare the global look-up on every loop iteration.
Yes

Originally Posted by Rainrider View Post
If it would be a local function call, or a local table look-up or a literal, I shall declare the variable for this in the loop. Is this the right way to sum it up?
If the variable is dependent on i in some way then define it in the loop, otherwise it would be defined outside as its value will not change during the loop.
  Reply With Quote
08-24-13, 04:39 PM   #12
SDPhantom
A Pyroguard Emberseer
 
SDPhantom's Avatar
AddOn Author - Click to view addons
Join Date: Jul 2006
Posts: 2,313
Originally Posted by Rainrider View Post
What is the difference between "needed in all iterations" and "needed in each independent iteration"?
To explain this simply, every single time code in a loop is run is considered a single iteration of the loop. All locals defined inside a loop only exist in a single iteration independent of all the others that may be run. As an example, a loop that runs code 5 times has 5 iterations whose scope is independent of each other.
__________________
WoWInterface AddOns
"All I want is a pretty girl, a decent meal, and the right to shoot lightning at fools."
-Anders (Dragon Age: Origins - Awakening)
  Reply With Quote
08-25-13, 01:42 AM   #13
myrroddin
A Pyroguard Emberseer
 
myrroddin's Avatar
AddOn Author - Click to view addons
Join Date: Oct 2008
Posts: 1,240
To illustrate:
Lua Code:
  1. for i = 1, 5 do
  2.     -- all iterations
  3.     local var = "dog"
  4.     print(var)
  5. end
  6.  
  7. -- var never needs to be changed
  8. local var = "dog"
  9. for i = 1, 5 do
  10.     print(var)
  11. end
In the first example, you create five times the amount of garbage collected compared to the second.
  Reply With Quote
08-25-13, 04:02 AM   #14
Malsomnus
A Cobalt Mageweaver
AddOn Author - Click to view addons
Join Date: Apr 2013
Posts: 203
Originally Posted by myrroddin View Post
To illustrate:
Lua Code:
  1. for i = 1, 5 do
  2.     -- all iterations
  3.     local var = "dog"
  4.     print(var)
  5. end
  6.  
  7. -- var never needs to be changed
  8. local var = "dog"
  9. for i = 1, 5 do
  10.     print(var)
  11. end
In the first example, you create five times the amount of garbage collected compared to the second.
This is why I can't quite figure out the general statement "define variables in the lowest scope possible"
__________________
SanityCheck - If you've ever said the words "Sorry, I forgot" then you need this add-on.

Remember, every time you post a comment on an add-on, a kitten gets its wings!
  Reply With Quote
08-25-13, 06:00 AM   #15
Phanx
Cat.
 
Phanx's Avatar
AddOn Author - Click to view addons
Join Date: Mar 2006
Posts: 5,617
Originally Posted by Malsomnus View Post
This is why I can't quite figure out the general statement "define variables in the lowest scope possible"
Here's an example:

Code:
local unit
for i = 1, 4 do
    unit = "party"..i
    print(UnitName(unit), "is a", UnitClass(unit))
end
In this example, the "unit" variable is only used inside the "for" loop, and its value is dependent on the iterator's value, so it should only be defined in that scope, not in a higher scope:

Code:
for i = 1, 4 do
    local unit = "party"..i
    print(UnitName(unit), "is a", UnitClass(unit))
end
On a side note, in some cases, the narrowest scope necessary for a variable is no scope at all. I've definitely seen cases of this in addons:

Code:
local x = 4
for i = 1, x do
    local unit = "party"..i
    print(UnitName(unit), "is a", UnitClass(unit))
end
In that case, the variable "x" doesn't need to exist at all; you should just use its value directly in the loop construction, as in the previous example.
__________________
Retired author of too many addons.
Message me if you're interested in taking over one of my addons.
Don’t message me about addon bugs or programming questions.

Last edited by Phanx : 08-25-13 at 06:04 AM.
  Reply With Quote
08-25-13, 06:09 AM   #16
Phanx
Cat.
 
Phanx's Avatar
AddOn Author - Click to view addons
Join Date: Mar 2006
Posts: 5,617
Originally Posted by SDPhantom View Post
As far as using unpack() versus manually indexing tables, it depends on how you're indexing the table for each return and at some point, I would think unpack() would start to be more appealing, but no data exists to suggest at what point (if any) this will start to happen.
I did some benchmarking on this a while ago. I don't know whether t[a][b][1], t[a][b][2], t[a][b][3] is faster than unpack(t[a][b]) (though I'd guess it is) but I do know that t[1], t[2], t[3] is significantly faster than unpack(t), so I would strongly advise using SDPhantom's alternative instead of using unpack:

Originally Posted by SDPhantom View Post
Code:
local points=ns.colors.cpoints[i]
cPoint:SetTexture(points[1], points[2], points[3])
__________________
Retired author of too many addons.
Message me if you're interested in taking over one of my addons.
Don’t message me about addon bugs or programming questions.
  Reply With Quote
08-25-13, 06:42 AM   #17
Malsomnus
A Cobalt Mageweaver
AddOn Author - Click to view addons
Join Date: Apr 2013
Posts: 203
Phanx: Sorry, what I meant to say was not that I don't understand the statement, but that I don't understand the reasoning behind it, which appears counter-intuitive to me.
If we take your first and second examples, what do you gain by moving the variable into the loop? It seems like you just define 4 variables instead of just 1, which means more CPU time spent on creating the variable and more work for the GC.
__________________
SanityCheck - If you've ever said the words "Sorry, I forgot" then you need this add-on.

Remember, every time you post a comment on an add-on, a kitten gets its wings!
  Reply With Quote
08-25-13, 10:10 AM   #18
Lombra
A Molten Giant
 
Lombra's Avatar
AddOn Author - Click to view addons
Join Date: Nov 2006
Posts: 554
I won't comment too much on the performance aspect, but there's probably nothing to gain by doing it that way. It's possible that there's some minuscule performance loss, but it'll never ever be something that anyone will notice, much less worry about. Assigning the value to the variable is the significant portion of the code, not the declaration of the variable.

Anyway, the real reason is readability and consistency. If you have a variable that's only relevant in one iteration of a loop, it has no place outside of that loop. If you have a variable that doesn't change for the entire loop however, it makes sense to define that variable and assign the value only once.
__________________
Grab your sword and fight the Horde!
  Reply With Quote
08-25-13, 09:52 PM   #19
Phanx
Cat.
 
Phanx's Avatar
AddOn Author - Click to view addons
Join Date: Mar 2006
Posts: 5,617
Originally Posted by Malsomnus View Post
Phanx: Sorry, what I meant to say was not that I don't understand the statement, but that I don't understand the reasoning behind it, which appears counter-intuitive to me.
If we take your first and second examples, what do you gain by moving the variable into the loop? It seems like you just define 4 variables instead of just 1, which means more CPU time spent on creating the variable and more work for the GC.
You're creating 4 string values and performing 4 variable assignments either way. It doesn't matter whether you're overwriting an existing variable or not, so you should use scoping appropriately, and keep your code clean.
__________________
Retired author of too many addons.
Message me if you're interested in taking over one of my addons.
Don’t message me about addon bugs or programming questions.
  Reply With Quote
08-26-13, 03:57 AM   #20
kurapica.igas
A Chromatic Dragonspawn
Join Date: Aug 2011
Posts: 152
If an addon has A.lua(at the top of the toc file), B.lua

A.lua:
Code:
local addonName, addon = ...

if not getmetatable(addon) then
	setmetatable(addon, {
		__index = function(self,  key)
                        -- keep vars in the _G to the addon to reduce the access cost
			local v = _G[key]
			if v ~= nil then
				rawset(self, key, v)
				return rawget(self, key)
			end
		end,

		__metatable = true,
	})
end

setfenv(1, addon)

-- Standalone code environment, also all files in one addon share the same environment
function myFunc() end

print(_G.myFunc)  -- nil, you get print from _G, _G won't get myFunc from your addon
B.lua
Code:
local addonName, addon = ...

setfenv(1, addon)

myFunc()  -- you can call the function myFunc defined in A.lua
  Reply With Quote

WoWInterface » Developer Discussions » Lua/XML Help » About add-ons optimization

Thread Tools
Display Modes

Posting Rules
You may not post new threads
You may not post replies
You may not post attachments
You may not edit your posts

vB code is On
Smilies are On
[IMG] code is On
HTML code is Off