Quantcast
Recycling Tables, is it a good idea? - WoWInterface
Thread Tools Display Modes
07-23-16, 05:22 PM   #1
MuffinManKen
A Flamescale Wyrmkin
AddOn Author - Click to view addons
Join Date: Dec 2009
Posts: 105
Recycling Tables, is it a good idea?

I thought I'd recently read something about whether recycling tables in lua was a good idea or not, but despite a lot of searching I couldn't find it.

I'm maintaining an addon that has table recycling, and if it really is a performance improvement then I'm okay with leaving it, but otherwise I prefer simple code.

What is the current state of this practice?
  Reply With Quote
07-23-16, 05:26 PM   #2
Lombra
A Molten Giant
 
Lombra's Avatar
AddOn Author - Click to view addons
Join Date: Nov 2006
Posts: 554
You most likely don't have to (shouldn't) worry about it. If you're dealing with fairly large tables and/or frequently creating new ones it might be worth considering.
__________________
Grab your sword and fight the Horde!
  Reply With Quote
07-23-16, 05:41 PM   #3
semlar
A Pyroguard Emberseer
 
semlar's Avatar
AddOn Author - Click to view addons
Join Date: Sep 2007
Posts: 1,059
If the person who wrote it thought it was worth the extra effort to implement, it probably wouldn't be a good idea to take it out just to simplify the code, at least not unless you're quite certain it's redundant.
  Reply With Quote
07-24-16, 09:19 AM   #4
VincentSDSH
Non-Canadian Luzer!
 
VincentSDSH's Avatar
AddOn Author - Click to view addons
Join Date: Jun 2006
Posts: 350
Originally Posted by MuffinManKen View Post
I'm maintaining an addon that has table recycling, and if it really is a performance improvement then I'm okay with leaving it, but otherwise I prefer simple code.

What is the current state of this practice?
If my memory serves, early on in WoW, recycling was a must b/c the garbage collection in the Lua implementation was done at a threshold and usually caused a perceptible effect on the game. The implementation used now has incremental garbage collection so it has largely removed the need to constantly recycle tables. So if it's a very old addon, that may be the reason it recycles; if it's only a few years old, there might be a good reason it recycles.

However, w/o knowing the addon or the code or the logic behind it, it's impossible to say if it should be retained or not. There are still very good reasons to do it under certain circumstances.

So the generic answer is: if it isn't broke, don't fix it.
The generic answer if you want to do a bit if work is: do a version w/o it and then profile each.

Addons that use a lot of tables, esp large ones rebuilt for short durations, will tend to have some form of recycling but that most addons aren't complex enough to worry about it overmuch, but again it is very dependent on the addon.

Edit: I'm assuming Autobar is the addon. If so, I think it predates the incremental garbage collection so it'd recycle its tables. It's been years since I poked around trying to repair Autobar so I can't recall the kind of table usage it necessitates.
__________________
AddonsExecutive Assistant User Configurable To-Do ListLegible Mail Choose the Font for Your Mail

Last edited by VincentSDSH : 07-24-16 at 09:29 AM.
  Reply With Quote
07-24-16, 02:48 PM   #5
MuffinManKen
A Flamescale Wyrmkin
AddOn Author - Click to view addons
Join Date: Dec 2009
Posts: 105
Originally Posted by semlar View Post
If the person who wrote it thought it was worth the extra effort to implement, it probably wouldn't be a good idea to take it out just to simplify the code, at least not unless you're quite certain it's redundant.
I have spent many years working on legacy code and I have seen no shortage of cases which convince me of the opposite of your argument.

I'm pretty sure that regardless of whether this would be a good place to do it, this implementation simply doesn't work. Here is the "Recycle" function:

Code:
function Recycle.prototype:Recycle()
	table.insert(self.recycleList, trash)
end
That is the only mention of the "trash" variable in the entire code base. So I guess most of the time it's just pushing nil into the table, which I *think* basically does nothing other than waste time. Of course since it isn't scoped, if another addon happened to assign something to the global "trash" then all sorts of fun would happen!
  Reply With Quote
07-24-16, 02:53 PM   #6
MuffinManKen
A Flamescale Wyrmkin
AddOn Author - Click to view addons
Join Date: Dec 2009
Posts: 105
Originally Posted by VincentSDSH View Post
If my memory serves, early on in WoW, recycling was a must b/c the garbage collection in the Lua implementation was done at a threshold and usually caused a perceptible effect on the game. The implementation used now has incremental garbage collection so it has largely removed the need to constantly recycle tables. So if it's a very old addon, that may be the reason it recycles; if it's only a few years old, there might be a good reason it recycles.
Thank you, this was the sort of information I was looking for. I like to try to understand issues rather than just trying stuff and seeing what happens.

The generic answer if you want to do a bit if work is: do a version w/o it and then profile each.
Yeah, it generally comes down to that. I just wanted to understand the issue more before trying to decide whether I want to try to do that much work. If the answer was "WoW's GC is AWFUL, avoid at all costs", then I would approach it differently.

Edit: I'm assuming Autobar is the addon. If so, I think it predates the incremental garbage collection so it'd recycle its tables. It's been years since I poked around trying to repair Autobar so I can't recall the kind of table usage it necessitates.
Yes, it is AutoBar, so it's been around for quite a while and through at least 2 previous authors. Also, I believe it was partway through a rewrite when it was abandoned and I took over so I've found many parts that just don't work or were clearly never completed.
  Reply With Quote
07-24-16, 09:58 PM   #7
VincentSDSH
Non-Canadian Luzer!
 
VincentSDSH's Avatar
AddOn Author - Click to view addons
Join Date: Jun 2006
Posts: 350
Originally Posted by MuffinManKen View Post
Also, I believe it was partway through a rewrite when it was abandoned and I took over so I've found many parts that just don't work or were clearly never completed.
That explains the code fragment you mentioned above, I was going to ask when I saw it. It looks like the flotsam that gets left over from aborted refactors, and you're right, if it isn't set, it's just wasting time attempting to insert a null entry.

My gut feeling is that it should be removed at this point just out of safety.
__________________
AddonsExecutive Assistant User Configurable To-Do ListLegible Mail Choose the Font for Your Mail
  Reply With Quote

WoWInterface » Developer Discussions » Lua/XML Help » Recycling Tables, is it a good idea?

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