WoWInterface

WoWInterface (https://www.wowinterface.com/forums/index.php)
-   Lua/XML Help (https://www.wowinterface.com/forums/forumdisplay.php?f=16)
-   -   Efficient LUA code? (https://www.wowinterface.com/forums/showthread.php?t=21444)

shiin 03-26-09 02:06 AM

Efficient LUA code?
 
Since I am currently refactoring my old addons, I was wondering wether there are some guides out there on how to write efficient LUA code (i.e. the basic do and don'ts).

Zyonin 03-26-09 03:03 AM

I don't know about guides to efficient Lua code, however if you have an IRC client (they are easy to find and you may have one without even knowing it), then go over to:

irc.freenode.net

and join the channel:

#wowace

Then you can talk code with the various crazies over at WoWAce. Otherwise find some good examples of efficient code (Tekkub's mods are a good example, then again I am just a novice coder myself), take em apart and see how the author has put it together.

Another IRC channel is:

irc://irc.freenode.net/WoWUIDev

Eggi 03-26-09 04:04 AM

Don't focus on efficiency as it is absolutley not important if your addon uses a few kb more memory or a little bit more processor time. It is much more important that you write clean code.
Only if some parts of your addon are called very often (e.g. OnUpdate) you should try to avoid doing unnecessary things there.

Torhal 03-26-09 05:13 AM

Quote:

Originally Posted by Eggi (Post 122507)
Don't focus on efficiency... It is much more important that you write clean code.

In many cases, the two are mutually exclusive. :)

Slakah 03-26-09 05:15 AM

Heres a couple of links:

http://lua-users.org/wiki/OptimisationCodingTips
www.lua.org/gems/sample.pdf

The main sources of performance issues I've noticed are lack of table reuse, another way to help is to make your Options only load into memory when needed.

Zergreth 03-26-09 08:14 AM

I disagree with you, Eggi. Efficiency is one of the most important things to me. If everybody would code addons the way you say it, without looking at efficiency, then that would probably mutate into some large pile of badly used memory and processing time. I have to agree with Torhal's post, too. If you want clean code, you can as well go for efficiency, since efficient code is usually clean.

Tekkub 03-26-09 07:44 PM

Efficiency and clean code go hand in hand. You can write "beautiful" code that runs like ****, or amazingly fast code that is impossible to read. The real skill is doing both :P

seebs 03-26-09 07:45 PM

The key to efficiency is realizing that 90% of efficiency is writing the right design. No one cares how well your bubble sort avoids creating tables...

shiin 03-27-09 04:27 AM

Thanks for all the advice and the links. Some concrete concerns I was wondering about, is the use of string indices for table lookups and string comparisons, but apparently they are not as computational expensive in LUA than I expected them to be.

To name one concrete example, I have a buff module that listens for the UNIT_AURA event and then iterates over all buffs of the respective unit to see wether certain ones are active. For each one found, the number of its occurances is counted (e.g. count the number of Renews, Rejuvenations, etc.) and for certain buffs other actions are taken. Implementing the behaviour like this

for i=1, MAX_BUFFS do
buff_name = UnitBuff( unit, i );
if( not buff_name ) then break; end

if( watched_buffs[ buff_name ] ) then
found_buffs[ buff_name ] = found_buffs[ buff_name ] + 1;
end

if( buff_name == BUFF1 ) then
elseif( buff_name == BUFF2 ) then
elseif( buff_name == BUFF3 ) then
end
end

apparently is efficient in LUA.

Torhal 03-27-09 04:59 AM

Slight problem:

Code:

        if( not buff_name ) then
                break;
        end

This will cause the loop to exit whenever a buff name is not found, so if for example the first buff doesn't exist it will never check for the others.

Eggi 03-27-09 05:30 AM

Quote:

Originally Posted by Torhal (Post 122743)
Slight problem:

Code:

        if( not buff_name ) then
                break;
        end

This will cause the loop to exit whenever a buff name is not found, so if for example the first buff doesn't exist it will never check for the others.

As soon as there is a buff without a name he can stop it because then there are no more buffs left (I don't think that there is a buff that has no name).

Tekkub 03-27-09 11:36 AM

if not UnitBuff("player", "Mark of the Wild") then....

Gello 03-27-09 02:30 PM

Some days I'm partial to the Rowne Programming Elegance By Fiat method: "I touched that, therefore there is no more beautiful or efficient piece of code ever created. Behold! The laws of space and time warp to increase your fps despite my once-a-frame SetPoints! You may begin your praise now, I am ready to receive it."

Don't forget the many profiling tools Blizzard provides for us:
http://www.wowwiki.com/World_of_Warc...ging_Functions
http://www.wowwiki.com/World_of_Warc...ling_Functions

If you're unsure whether one way is more efficient than another, try doing it 100,000 times and seeing how long it took.

Tekkub 03-27-09 02:32 PM

Ah Rowne... no one thought they wrote more perfect code than he did...

seebs 03-27-09 05:04 PM

Now I have to see this code.

watchout 03-29-09 08:43 AM

Concerning string/hash indexes vs. numerical indexes

Lua puts any numerically indexed table entries into an array as opposed to a hashed map (every lua table can have both array data and hashed data).
So in theory, accessing numbered entries should be faster (adding items however could be another thing)

Now, to contradict my previous paragraph... Some time ago I did some testing of numeric vs. hash indexes (in plain lua.exe) and I was not able to determine any difference at all. Any time differences (at some 100k iterations) were well in threshold margins.

Some day I might also read into Lua's hashing function, but until then I'd say you try to have shorter indexes so the function has less characters to parse.
Also Lua might fall back to hashing when you have a table like {[1] = ..., [10000] = ...}


One thing to also note is: Avoid global tables at all costs. WoWs current global table is so overpopulated that you're almost always faster when you first do local x = sometableinglobal; and then do your operations on x. You're even faster if you completely skip the global table and stay within your addon's scope (have a table where everything is inside that you can access using self, or use local-to-file variables if possible).
Also note that accessing anytable.number is always slower than accessing number without a table (100-1000 times or so)

also note that your line
Code:

buff_name = UnitBuff( unit, i );
will put buff_name into the global table (unless you have localized it somewhere outside of the code piece) which (as stated above) is hideously slow, especially in wow, since overpopulated. Do
Code:

local buff_name = UnitBuff( unit, i );
instead

Assuming that you have more than three "elseif( buff_name == BUFF3 )"-like branches, you can further reduce your code - if you use a metatable that returns an empty function when an empty index is accessed, and do the stuff you'd now do inside your "if buff_name == xyz then" constructs in functions defined in that table - to:

Code:

local i=1;
local buff_name = UnitBuff( unit, i );
while buff_name do
        watched_buffs[buff_name]();
        i=i+1;
        buff_name = UnitBuff( unit, i );
end

or
Code:

local i=1;
local buff_name = UnitBuff( unit, i );
while buff_name do
        watched_buffs[buff_name]();
        found_buffs[buff_name] = found_buffs[buff_name]+1;

        i=i+1;
        buff_name = UnitBuff( unit, i );
end

(using a 2nd metatable that discards any set if there's a new index and returns zero if an unknown index is requested, thats a pretty overhead though) if you really need that counting table.
What both options have in common is that they're slower when the buff is not in the table and really fast when it is, because then the metatable methods are not used.

The reason to do this is, because in an if/elseif/...../else construct lua has to parse every single branch in the worst case - half of them in the average case - and if you call the table it just does a hash of the string, goes there in the table (with native speed!) and calls that code (with some overhead due to function header).

Aezay 03-29-09 11:43 AM

Quote:

Originally Posted by watchout (Post 123025)
One thing to also note is: Avoid global tables at all costs. WoWs current global table is so overpopulated that you're almost always faster when you first do local x = sometableinglobal; and then do your operations on x. You're even faster if you completely skip the global table and stay within your addon's scope (have a table where everything is inside that you can access using self, or use local-to-file variables if possible).
Also note that accessing anytable.number is always slower than accessing number without a table (100-1000 times or so)

I just did a test after logging in, meaning most addons wouldn't have loaded their dynamic or load on demand parts yet, and my global table has 32047 entries. Would be quite interesting to know what the size of the global table actually does to performance. Though most addons are written to use locals, and shouldn't be affected by this, it would slow down the default interface code, which makes *extreemly heavy* use of globals and global function calls like getglobal.

Many things in my UI is my own code though, and I try to pollute the global namespace as little as possible, only adding one global, or in some cases none, and I know many other addon authors will do the same.

You learn a lot of good coding guidelines from this community, this thread is a good example. I just think it is funny that the Blizzard coders aren't learning from this too, their use of globals is frightening, when we know how slow it is compared to locals.

To test how many globals your interface has, use this code:
Code:

/run c = 1; for _ in next, _G do c = c +1 end print(c)

Foxlit 03-29-09 02:16 PM

Quote:

Originally Posted by watchout (Post 123025)
One thing to also note is: Avoid global tables at all costs. WoWs current global table is so overpopulated that you're almost always faster when you first do local x = sometableinglobal; and then do your operations on x. You're even faster if you completely skip the global table and stay within your addon's scope (have a table where everything is inside that you can access using self, or use local-to-file variables if possible).

The number of entires in the global environment has nothing to do with this. There's generally no difference in access time between "large" and "small" hash tables -- the number of buckets is simply increased when more entires are added, which is a one-time cost when the table is rehashed. The entire "overpopulated table" line of reasoning is invalid.

The performance difference you observe is caused by only looking up "sometableinglobal" once, rather than every time you access it. This applies to *all* nested tables: if you're performing a large number of operations on some sub-table, use a local reference rather than looking it up in its parent table every time.

watchout 03-30-09 08:06 PM

Quote:

Originally Posted by Foxlit (Post 123054)
The number of entires in the global environment has nothing to do with this. There's generally no difference in access time between "large" and "small" hash tables -- the number of buckets is simply increased when more entires are added, which is a one-time cost when the table is rehashed. The entire "overpopulated table" line of reasoning is invalid.

Somehow I'd hoped such an answer would come... though I thought the part about meta-tables would cause more replies.

Yes, size does matter:banana:
O(1) (for lookup) is only average case for your typical hash table implementation, and that only if the key-hashes follow the perfect distribution that the hash-function designer thought to be universal. Worst case is O(n). Reality will be somewhere in between, especially since best case O(1) = average case O(1) should give you a hint on why the O-notation is not always the perfect method to judge the performance of an algorithm.

Quote:

Originally Posted by Foxlit (Post 123054)
The performance difference you observe is caused by only looking up "sometableinglobal" once, rather than every time you access it. This applies to *all* nested tables: if you're performing a large number of operations on some sub-table, use a local reference rather than looking it up in its parent table every time.

This however... is wrong. In the specific way that you did not correctly guess what performance difference I observed. The performance difference you mean is because global variables are stored in a hashtable, while locals are stored / used in the registers. True. I definitely don't object to that, but that's too obvious.
I was telling from old data, the difference used to be much bigger back in 2.x. I reran my bench when writing this post and found that the performance of the global table has significantly improved (it's still not completely the same as an empty table though, about 2-5% off *cough*), I apologize for this. I just should have rerun the bench when writing my previous post.



@Aezay:
It used to be much bigger - so big actually that my client locked up and disconnected before finishing counting. After that I stopped trying... I think I read something about blizzard removing global variables in some patch notes though.
Your count is one off btw. you need to start at zero ;)

Tekkub 03-30-09 08:40 PM

****, someone had to bring up O notation

*glazes over*

Foxlit 03-31-09 02:03 AM

Quote:

Originally Posted by watchout (Post 123219)
O(1) (for lookup) is only average case for your typical hash table implementation, and that only if the key-hashes follow the perfect distribution that the hash-function designer thought to be universal.

You might have a point if you can show that hash collisions occur more frequently in the global environment than they would in your small table, taking into account the difference in the amount of allocated hash buckets. Personally, I don't think that's the case.

Quote:

Worst case is O(n). Reality will be somewhere in between, especially since best case O(1) = average case O(1) should give you a hint on why the O-notation is not always the perfect method to judge the performance of an algorithm.
I don't see a problem with the O-notation here.

Quote:

This however... is wrong. In the specific way that you did not correctly guess what performance difference I observed. [...] I reran my bench when writing this post and found that the performance of the global table has significantly improved (it's still not completely the same as an empty table though, about 2-5% off *cough*), I apologize for this. I just should have rerun the bench when writing my previous post.
Unfortunately, I can only guess at what testing you're doing without looking at the code. A 2-5% difference seems small enough to be caused by some form of noise.

Quote:

It [the number of entries in the global environment] used to be much bigger - so big actually that my client locked up and disconnected before finishing counting.
Iterating over a table with one million string keys and incrementing a counter each iteration takes slightly over 0.3s using lua on my laptop. It's possible that WoW's Lua implementation, or your PC (or both) are slower than what I'm testing against; on the other hand, I doubt the difference is much greater than 1:10. Extrapolating on that, it'd take at least several million entries to freeze your client long enough to force a disconnect. The most obvious conclusion is that your counting code did something other than just count entries.

Aezay 03-31-09 06:14 AM

Now that we are already talking about "efficient" Lua code, I have a few things I've been wondering about lately.
String concats, I know they are bad to do many times as it will create a new sting each time, but I was curious which one of these lines would be the best to use? Does a concat of an empty string mean much?
Code:

label:SetText(FormatTime(timeLeft)..(showTotalTime and " / "..duration or ""));
label:SetText(showTotalTime and FormatTime(timeLeft).." / "..duration or FormatTime(timeLeft));

Is there any performance benefits in assigning multiple variables at the same times compared to one per line?
Code:

tbl.name, tbl.value, tbl.color = name, value, color;

tbl.name = name;
tbl.value = value;
tbl.color = color;

Also, what is this O-notation you're talking about?

watchout
Doh, you're right, ah well, not like it really means anything :rolleyes:

Slakah 03-31-09 08:49 AM

Quote:

Originally Posted by Aezay (Post 123264)
Is there any performance benefits in assigning multiple variables at the same times compared to one per line?
Code:

tbl.name, tbl.value, tbl.color = name, value, color;

tbl.name = name;
tbl.value = value;
tbl.color = color;


Code:

local clock = os.clock
local insert, sort = table.insert, table.sort

local orderednames = {}
local names = setmetatable({}, {__newindex = function(t, k, v)
    insert(orderednames, k)
    rawset(t, k, v)
end})

local function bench(name, func, ...)
    local start = clock()
    for i = 1, 1e7 do
        func(...)
    end
    names[name] = clock() - start
end

local function printresults()
    sort(orderednames, function(a, b)
        return names[a] < names[b]
    end)

    for i, v in pairs(orderednames) do
        print(string.format("%d  %s took %.5f secs", i, v, names[v]))
    end
end

local tbl = {}
tbl.cat, tbl.dog, tbl.cow, tbl.pig = "Meow", "Woof", "Moo", "Oink"


bench("1 line assign", function()
    tbl.cat, tbl.dog, tbl.cow, tbl.pig = "Meow", "Woof", "Moo", "Oink"
end)

bench("multi line assign", function()
    tbl.cat = "Meow"
    tbl.dog = "Woof"
    tbl.cow = "Moo"
    tbl.pig = "Oink"
end)

printresults()

I'm consistently getting these results
Code:

1  multi line assign took 11.77000 secs
2  1 line assign took 12.55000 secs

I can't explain it.

Aezay 03-31-09 10:42 AM

I completely forgot I had a lua5.1.exe lying around, so I just ran your code and come up with this, same results really, just faster. What did you use to run the code in?
Code:

1  multi line assign took 2.01600 secs
2  1 line assign took 2.54600 secs


Using your benchmark code, I tried running this to see which was fastest. It is more or less a copy/paste from one of my addons, from an OnUpdate script on a timer.
Code:

local text;
local showTotalTime = false;
local timeLeft, endTime;
local duration = 60;

local function FormatTime(sec)
        if (abs(sec) <= 60) then
                return string.format("%.1f",sec);
        else
                return string.format("%d:%.2d",sec / 60,abs(sec) % 60);
        end
end

endTime = clock() + duration;
bench("empty string concat",function()
        timeLeft = (endTime - clock());
        text = FormatTime(timeLeft)..(showTotalTime and " / "..duration or "");
end);

endTime = clock() + duration;
bench("no concat",function()
        timeLeft = (endTime - clock());
        text = showTotalTime and FormatTime(timeLeft).." / "..duration or FormatTime(timeLeft);
end);

To my surprise it didn't really matter which way it's done, I guess that is because the concat of an empty string is on the same line as the FormatTime call, and thus is done in the same go, meaning it wont actually create a new Lua string.
Code:

1  no concat took 13.15700 secs
2  empty string concat took 13.21800 secs

Setting the showTotalTime to true produces these results, which again is pretty much the same.
Code:

1  no concat took 23.56300 secs
2  empty string concat took 24.56200 secs


Slakah 03-31-09 11:23 AM

Quote:

Originally Posted by Aezay (Post 123290)
What did you use to run the code in?

I'm on my netbook, so that's to be expected :).

Aezay 03-31-09 11:26 AM

Just updated my post above, while you posted, with some tests about string concats.

What about meant with where you ran your code, was which program, was it also lua5.1.exe?

Slakah 03-31-09 11:49 AM

Yeah I'm using lua 5.1 on Fedora Core 10, on an Asus Aspire One.

Quote:

Also, what is this O-notation you're talking about?
I was also interested so I found this http://www.perlmonks.org/?node_id=227909.

So if my understanding is correct, if a table lookup is O(1) then the size of the table doesn't affect the speed of the lookup?

IBLJerry 03-31-09 01:00 PM

O(1) means that the time taken to perform an algorithm is a constant, whatever the input size. O(n) means that the time is a multiple of the input size (plus a possible constant).

Algorithm complexity is interesting as a tool to compare how algorithm scale with the input size. But what this tool doesn't tell you, is which algorithm is the fastest. An O(1) algorithm that takes one hour to complete is worthless compared to an O(n) algorithm that takes seconds to do the same for reasonable input sizes.

Aezay 03-31-09 01:32 PM

Thanks for the explanation IBLJerry.

I'm a little curious though, can anyone give an example of an algorithm that doesn't scale with the data?

Zergreth 03-31-09 01:47 PM

"Determining if a number is even or odd; using a constant-size lookup table or hash table" is the example delivered by Wikipedia.

Shadowed 03-31-09 04:46 PM

You guys are vastly over-complicating this, from a fun abstract random curiosity type of question sure, but for practical applications, all you really need to ask yourself is "Am I doing something really stupid?" The majority of the mistakes I see new people make are they create a static table every time a function is loaded instead of once on file load like you see below. Creating options tables only when needed instead of all the time as Slakah mentioned as well.

Code:

function foo()
    local staticColor = { r = 1.0, g = 1.0, b = 1.0 };
end

Where you can shift it up so it becomes

Code:

local staticColor = { r = 1.0, g = 1.0, b = 1.0 };
function foo()
end

So you only create one table instead of the same table every time the function is called.

Eggi touched on this, but always ask yourself what the function is doing. If you're writing a mod for checking if the player has a buff and warning them, efficiency is a good goal but you shouldn't worry about it; on the other hand, if you're writing a mod to monitor the auras of 25 people in a raid during combat, then efficiency should be something you care more about. If you it requires user interface (GUIs, and that kind of thing) or they can't do it in combat then efficiency should be a lot lower on your list.

watchout 03-31-09 07:31 PM

More tips:
  • Turn off what you don't need. Think about whether you really always need those 1000 lines of OnEvent/OnUpdate function. Maybe you can split it into smaller functions that you switch on rare events? (like entering/leaving combat or switching gear, etc.)
  • reuse your frames
  • maybe split multiple events on multiple frames so that C-code handles the distribution and you don't have to manually check if it's actually the right event when the handler is called
  • Strings generate their hashes when created - that's pretty fast but it can't hurt to keep string generations at a minimum.
  • Use numbers over strings where possible.
  • Don't create functions that are called only directly (that means not through some table like I did in my example above) and in one place.
  • Memory usually comes cheap, while processor time is expensive.
@Aezay: of course, I just saw it and... just... had to say it... aaah the dark side of the force is beckoning!

Aezay 04-02-09 03:05 PM

If I want to store a list of strings, let's say for some kind of filter, what is the fastest way to do it, store them as keys, or as values?
Storing them as keys looks better code wise, but that doesn't mean it is faster. But based on the previous talk in this thread, I assume that keys is just as fast, if not faster?
Code:

if (tableFilter[name]) then
        frame:Show();
end

Code:

for index, entry in ipairs(tableFilter) do
        if (name == entry) then
                frame:Show();
                break;
        end
end


Tekkub 04-02-09 03:09 PM

Keys are always faster for that sort of thing. Iterating over the table makes a bunch of function calls.

IBLJerry 04-03-09 03:24 AM

Quote:

Originally Posted by Tekkub (Post 123811)
Keys are always faster for that sort of thing.

Unless you want to sort the keys :-)

Tekkub 04-03-09 03:56 AM

Which wouldn't be "that sort of thing" (finding out if the value is in the talbe)

Aezay 04-04-09 03:39 PM

To save a function call, many uses next() as the iterater in a for loop, why not do the same for ipairs(), should be easily obtainable using this code. Or is there an iterator function already available for array type tables, like next?
This would probably be quite silly normally, unless it's code run often like in an OnUpdate script.

Code:

local iterator = ipairs(_G);

Tekkub 04-04-09 03:46 PM

You'd only save one function call out of all the ones made while looping. It'd be better to optimize so that you don't need to iter at all.

Shadowed 04-04-09 03:46 PM

Quote:

Originally Posted by Aezay (Post 124195)
To save a function call, many uses next() as the iterater in a for loop, why not do the same for ipairs(), should be easily obtainable using this code. Or is there an iterator function already available for array type tables, like next?
This would probably be quite silly normally, unless it's code run often like in an OnUpdate script.

Code:

local iterator = ipairs(_G);

I think I've seen maybe one or two people use next, I'm not sure I would call it common at all. This gets into premature/unnecessary optimization

shiin 04-14-09 07:27 AM

Regarding events, I was always wondering would be the better coding style:
a) Catching all events of one type (i.e. UNIT_BUFF) in one central location and distributing them to the relevant frames (i.e. RaidFrameNumberXY),
b) Having decentralised event handlers for each frame, each listening for all relevant events.

Evidently, variant b) uses more CPU time, since each event would have to be analysed once for each frame, but having frames as independent building blocks seems to be more manageable and extendable.

Tekkub 04-14-09 05:04 PM

Totally depends on your design. If you already have the frames for display, and the related events/handlers are simple, it's probably best to assign them directly to the frame and make good use of self. If you don't have the frames, a central dispater is fine if you write it well. You can make a dispatcher in a single line of code that works wonderfully...

f:SetScript("OnEvent", function(self, event, ...) if self[event] then return self[event](self, event, ...) end end)


All times are GMT -6. The time now is 07:52 PM.

vBulletin © 2024, Jelsoft Enterprises Ltd
© 2004 - 2022 MMOUI