Remove all objects

@p120ph37: Just for clarification, your new group:removeSelf() method should replace a need for a cleanGroups() function, and be able to effectively “clean out” a group just by calling:

myGroup:removeSelf()

Is that correct?

Thanks again! [import]uid: 7849 topic_id: 7489 reply_id: 26738[/import]

Thanks, its all good now. [import]uid: 35210 topic_id: 7489 reply_id: 26756[/import]

@jonbeebe: yes, that’s the idea exactly. [import]uid: 32962 topic_id: 7489 reply_id: 26770[/import]

The solution where you substitute the global display.newGroup() with the patched-one may not work always.

In many libraries you see the pattern:

[lua]local newGroup = display.newGroup[/lua]

(I just used newGroup here to make the point)

Some even recommend this pattern as a best practice because it forces you to localize all the symbols that you use in your module. Leaking globals that pollute and overwrite the Lua namespace is a nasty problem.

If you use this localization, however, then it depends on when you do the patching and when do you require the module.

For example, take the following file “monkey.lua”:
[lua]local newGroup = display.newGroup
print(“monkey.newGroup begin:”, newGroup)

local monkey = {}

monkey.newGroup = function(…)
print(“monkey.newGroup:”, newGroup)
return newGroup(…)
end

return monkey[/lua]

and add a require(“monkey”) before the newGroup-patch, and substitute the first display.newGroup in the event handler with monkey.newGroup:

[lua]local monkey = require(“monkey”)

local oldNewGroup = display.newGroup
print(“oldNewGroup:”, oldNewGroup)
display.newGroup = function( … )
local group = oldNewGroup( … )
local oldRemoveSelf = group.removeSelf
group.removeSelf = function( self )
for i = self.numChildren, 1, -1 do
self[i]:removeSelf()
end
if(self.name==“ui”)then print(“newRemoveSelf”, self) end
oldRemoveSelf( self )
end
group.remove = function( self, o )
if type( o ) == ‘number’ then
self[o]:removeSelf()
else
o:removeSelf()
end
end
return group
end
–]]

– patch stage object to proxy stage:remove( o ) to o:removeSelf()
display.getCurrentStage().remove = function( self, o )
if type( o ) == ‘number’ then
self[o]:removeSelf()
else
o:removeSelf()
end
end

print(“new newGroup:”, display.newGroup)

local NamedObjects = require(“NamedObjects”)

Runtime:addEventListener(‘enterFrame’, function()
print(“new newGroup:”, display.newGroup)
NamedObjects.newGroupRef()
local g0 = monkey.newGroup()
local g1 = display.newGroup(); g0:insert(g1)
local g2 = display.newGroup(); g0:insert(g2)
local g3 = display.newGroup(); g0:insert(g3)
local r1 = display.newRect( 10, 10, 25, 25 ); g1:insert(r1)
local r2 = display.newRect( 20, 20, 25, 25 ); g1:insert(r2)
local r3 = display.newRect( 30, 30, 25, 25 ); g1:insert(r3)
local r4 = display.newRect( 40, 40, 25, 25 ); g2:insert(r4)
local r5 = display.newRect( 50, 50, 25, 25 ); g2:insert(r5)
local r6 = display.newRect( 60, 60, 25, 25 ); g2:insert(r6)
local r7 = display.newRect( 70, 70, 25, 25 ); g3:insert(r7)
local r8 = display.newRect( 80, 80, 25, 25 ); g3:insert(r8)
local r9 = display.newRect( 90, 90, 25, 25 ); g3:insert(r9)
– clear all
local stage = display.getCurrentStage()
for i = stage.numChildren, 1, -1 do
stage:remove(i)
end
– watch for leaks
collectgarbage()
print(collectgarbage(‘count’))
end)[/lua]

You will see the memory leak clearly happening on the console. If you move the require call past the patch code, all is good again.

In other words, the newGroup-patch may work most of the time and probably every time if you can ensure that the patch gets applied before any other module is loaded.

For me, the only real, fool-proof solution is if Ansca would hard-code the recursive removeSelf. Personally I cannot think of any use cases where you would want to hold on to display objects after the group they were in was deleted… and even if you have those cases where people want that, they should be forced to move their objects out of that group and put them in another before the group is removed.

Ansca are you listening? :wink: Would appreciate your take on this…

-FrankS.

[import]uid: 8093 topic_id: 7489 reply_id: 26781[/import]

Yes, in order for my method to be effective it ought to be included as the first “require” in main.lua, otherwise there is the risk that another library may localize the globals it patches before it can patch them.

This of course means that whoever is writing the code in main.lua needs to be aware of this dependence on the order of the requires, though I think for most scenarios this is not too large a burden to place on the coder, especially when the benefit is that misbehaving / leaking code may suddenly begin working as expected with no other change.

I’ve also done something similar with the Corona Remote code - I have created a library which patches the system globals and throws Runtime events so that the remote functionality can be patched transparently into an existing app which was written against the Corona accelerometer API with no code changes needed, rather than the typical method of writing your app specifically for remote.lua’s API. (I can share that code too if anyone is interested)

My day job involves Perl, so I feel no remorse in twisting the internals of a dynamic language to do my bidding while remaining deceptively straightforward on the surface :wink: One (of the many) mottos of Perl is “Do what I mean, not what I say.” [import]uid: 32962 topic_id: 7489 reply_id: 26853[/import]

Well… in a distant past I too have coded a fair amount of perl prose/poetry, and I’m so happy that that particular company went belly-up such that there is not even a remote chance that I’d be asked to debug any of my old scripts :wink:

The interesting fact about those monkey patches is that the one who writes it will easily use it with confidence, while those that have to adopt it are more than weary and suspicious to have it included in any module/library that is loaded…

Now being pragmatic, I am only interested in the consequences of adopting a solution like this, and as I understand it those are:

* this patching of display.newGroup() overwrites the global reference to the original Corona function, it wedges itself in between and all your libraries/modules will use it whether you like it or not (just a clarification to any readers that didn’t catch on to that yet)

* this global display.newGroup() patch must be included in the main.lua file before any other module is loaded to be effective and predictable (this will circumvent that competing localizing scenario that I described).

* God or any other higher coding-authority forbid that there is any other module loaded that was written by someone who had the great idea of also patching that same global display.newGroup() - all bets are off what would happen then… (although it could lead to some interesting bugs…:wink:

* Ansca should step up to make this issue go away - the idea that many of us would start to rely on this monkey-patching of their core library routines to sidestep memory-leak issues, should make them shiver as it makes future changes/fixes more complicated - Ansca are you reading this? Please engage in these discussions…

-FrankS.
[import]uid: 8093 topic_id: 7489 reply_id: 26865[/import]

I posted this to my blog, as well as a download to a script that everyone can include in their projects, that provides both solutions proposed by FrankS and p120ph37.

More details and download here:
http://jonbeebe.tumblr.com/post/3760389212/proper-group-cleaning-in-corona-script-download [import]uid: 7849 topic_id: 7489 reply_id: 27172[/import]

Nice blog entry and thanks for the acknowledgement.

However, I believe that you missed the point in the blog that neither of the solutions nor the combination is the best one, and that only Ansca can (and should) provide an enhanced removeSelf replacement for their Corona-sdk that would benefit everybody all the time tremendously as memory-leaks are about the nastiest critters in your code.

Regards, FrankS.
[import]uid: 8093 topic_id: 7489 reply_id: 27221[/import]

I appreciate your attempt to credit both FrankS and me, but I think your combination of the two methods is redundant - either one will suffice to clean out a group without the aid of the other, they just take different approaches to it.

Also, you threw in a lot of Scale=0.1 lines without explaining why. I imagine you did that in the hopes of shaking up any image objects enough that the other currently-at-large memory leak might not manifest, but you didn’t explain.

Otherwise, nice writeup. :slight_smile: [import]uid: 32962 topic_id: 7489 reply_id: 27236[/import]

Hey guys!
Looks great
Beebe I can’t get the script to work.
When I’m calling cleanGroup I get:
… attempt to call global ‘cleanGroup’ (a nil value)

What stupid mistake am I’m doing? :slight_smile:
I’m using it together with director and I’m require before everything else. [import]uid: 10657 topic_id: 7489 reply_id: 27252[/import]

@rickwhy: Please download the script again, I updated the download link. In the last module, I accidentally set the cleanGroup function to local out of habit.

You can either download the updated script here: http://cl.ly/58iT

Or you can simply remove ‘local’ from the cleanGroup() function definition in cleangroup.lua.

Sorry about that! All is fixed now though… [import]uid: 7849 topic_id: 7489 reply_id: 27254[/import]

awesome just found that in the other thread :slight_smile:
thanks [import]uid: 10657 topic_id: 7489 reply_id: 27255[/import]

jonbeebe, Thanks for the code.
But why are you scale object to 0.1? [import]uid: 9058 topic_id: 7489 reply_id: 27258[/import]

According to this thread:

http://developer.anscamobile.com/forum/2010/12/08/memory-leak-when-removing-and-adding-image-sequencely

The person was noticing a memory leak (that I’ve also experienced) in regards to removing a lot of images in a short amount of time (which is what cleanGroup()) does…

A strange thing they noticed is that strangely, the leak would not occur *sometimes* if the image was scaled down (weird, I know).

So I thought, since scaling an image right before removing it doesn’t take a lot of overhead, if it even potentially reduces the occurrence of memory leaks, why not include it in with the function? The worst that could happen is absolutely nothing, the best that could happen is that a potential leak could get squashed (due to an unknown bug at this time).

Great question though! I was wondering how many people would ask me that (I’ve gotten a few already, lol). [import]uid: 7849 topic_id: 7489 reply_id: 27291[/import]

It has been mentioned a few times that removing a group doesn’t remove the Display Objects within the group. This was an issue six months ago but the object:removeSelf() was fixed to clear texture memory used by display objects.

I created the following test code to demonstrate that removing a group does remove the texture memory used by the display objects within a group.

print( "Texture Memory: " .. system.getInfo( "textureMemoryUsed" ) )  
  
local image  
  
local group = display.newGroup()  
image = display.newImage( "Icon.png", 50, 0, true ) -- image 1  
group:insert( image )  
  
image = display.newImage( "Icon1.png", 50, 100, true ) -- image 2  
group:insert( image )  
  
image = display.newImage( "Icon2.png", 50, 200, true ) -- image 3  
group:insert( image )  
  
image = nil  
  
-- The group now contains three images  
  
-- Use a timer to display the texture memory and finally remove the group  
local function onTimer( event )  
 print( event.count .. ") Texture Memory: " .. system.getInfo( "textureMemoryUsed" ) )  
  
 if event.count == 1 then  
 print( "Removing group" )  
 group:removeSelf()  
 end  
end  
  
timer.performWithDelay( 500, onTimer, 2 )  

You can put the above code into a main.lua file and duplicate and rename the Icon.png files to see the results.

What the code shows is the texture memory consumed for the three images and the memory going to 0 after the group was removed. This was run on build #311.

Removing a group or removing a Display Object removes all the texture memory used by the Display Objects and converts the objects into normal Lua tables. Any non-display properties added to the objects will still exist, including listener functions. If there are no other references to the objects, Lua’s garbage collection should clean up the objects when it cleans other Lua variables.

The main purpose of group:remove and object:removeSelf methods is to remove the texture memory associated with display objects. Best practices say you should removed all object listeners and nil out the object if you still have access to the objects.

I’m also curious about scaling the display object before removing. Is there any test code that shows this is a real fix for memory leaks? Also, any test code showing memory problems would be helpful.

Update: It turns out the “scaling” mentioned as a cure for small memory leaks when removing a group/object was related to a touch bug that has been fixed in the current daily build (#311). See more here: http://developer.anscamobile.com/forum/2010/12/08/memory-leak-when-removing-and-adding-image-sequencely#comment-27408

Thanks,
Tom [import]uid: 7559 topic_id: 7489 reply_id: 27365[/import]

@Tom

Although texture memory is freed, there is still something on the Lua side that does not get freed automatically when a group is removed. I took your example code and modified to to hilight this:

[lua]local group

– Use a timer to create and remove a group and wait for GC to occur
– and display memory stats at each iteration.
local function onTimer( event )
if event.count % 3 == 1 then
print( “Creating group” )
local image
group = display.newGroup()
image = display.newImage( “Icon.png”, 50, 0, true ) – image 1
group:insert( image )

image = display.newImage( “Icon1.png”, 50, 100, true ) – image 2
group:insert( image )

image = display.newImage( “Icon2.png”, 50, 200, true ) – image 3
group:insert( image )

image = nil
– The group now contains three images
elseif event.count % 3 == 2 then
print( “Removing group” )
group:removeSelf()
group = nil
else
print( “It is now 500ms after the group was removed” )
end

collectgarbage() – collect any outstanding garbage that we can
print( event.count … ") Texture Memory: " … system.getInfo( “textureMemoryUsed” ) … " Lua memory: " … collectgarbage( “count” ) )

end

timer.performWithDelay( 500, onTimer, 21 )[/lua]

Note that the Lua garbage collector reports a steadily increasing memory usage. [import]uid: 32962 topic_id: 7489 reply_id: 27746[/import]

@p120ph37

When you did that, did you use your patched display.newGroup() function? [import]uid: 7849 topic_id: 7489 reply_id: 27747[/import]

Well, your right about the memory leak. Your code shows after seven group create/remove cycles there is a 1.8 KB memory leak. I extended the loop time beyond 21 but only did garbage collection/display and the memory leak never increased but it never recovered either.

I modified the code to only create the group once and remove the three images instead of removing the group and I saw no memory leak. That means removing the objects is not the issue.

I than added this code before removing the group:

 -- Remove three images  
 group:remove(1)  
 group:remove(1)  
 group:remove(1)  

That fixed the major portion of the memory leak problem. At the end I only saw a 1 or 2 byte memory leak.

The bottom line is removing the group does remove the children’s texture memory but it looks like there are still references remaining to the object variables which is not being cleaned up by Lua’s garbage collection.

The cleanGroups code that has been posted does remove all the children in the group which is the proper way to handle this issue.

I will update our bug database but I expect it will be considered a low priority because of the workaround. I will update our documentation with the new recommendation.
Thanks for posting this code so we could resolve the issue.
[import]uid: 7559 topic_id: 7489 reply_id: 27755[/import]

I would just like to add that Ansca should not rely on workarounds for known bugs, as the developers are not always get informed about these workarounds!

I am sure that the majority of the developers have absolutely no idea about this thread, the potential leaks and the workaround you are discussing. Personally, I came to this thread accidentaly.

A more formal mean of “temporary workarounds for known bugs” should be published and the developers should be formally aware of it, to keep themselves updated on critical issues. Maybe the newsletter should mention critical issues to be addressed, beyond the marketing material.

Regards [import]uid: 7356 topic_id: 7489 reply_id: 27763[/import]

@Magenda that’s a very good point that I will raise internally.

I have brought up the issue of “work-arounds” in the past and a way to publish them through a FAQ or other page.

There at a number of reported bugs and it hard to tackle them all at once so priority goes to show-stopper bugs and bugs with no work-around. We need to a better job of making these work-arounds better known.

At the very least, this information will be added to the “Remarks” section of the API page for the method/property involved.

Thanks,
Tom [import]uid: 7559 topic_id: 7489 reply_id: 27782[/import]