Destroying a BScreen

[perrce]

Using the TiVo HME SDK (1.4) and Bananas (1.3), if I pop a screen that I no longer need, and assuming that I'm not keeping a reference to it somewhere, should the BScreen object be destroyed? Or does the BApplication keep a reference to the BScreen somewhere out of my view?

I ask because I'm getting OutOfMemoryErrors when I browse lots of different screens (as when browsing through several music artists). A BScreen is created and is pushed, but I think that they may not be destroyed after they're popped.

Anyone have any ideas?

[s2kdave]

No the BApp doesn't hold the BScreen after it's popped. But your own objects might be. If you are using any non static inner classes or anonymous classes, those transparently hold a reference to the outer class. So until those object get garbage collected, the outer class will stick around.

An even more convoluted reference chain would be if you create an implementation of an interface and hand it off to something that is always present. And that interface has a reference to an object and that object has a reference to a non-static inner class of your screen and then your screen references a large chunk of data, then none of that stuff would get garbage collected.

Welcome to the exciting world of garbage collection. The easiest way to find memory leaks is to run a java profiler against your application to take heap snapshots.

[s2kdave]

oh, and when you use final variables they will stick around as long as you are in a scope that is able to reference it. So for example, if it's inside a method or on a method param, if you have an anonymous inner class in that method, the final variable sticks around until that anonymous inner class gets GC along with everything it references.

[tvengr]

Take this code from BrowseAlbumScreen:


new Thread() {
public void run() {
ImageResource albumArtImage;
if(thisAlbum.hasAlbumArt()) {
// Scale image to less of 640x480 or size of albumArtView.
// (640x480 is the maximum image size that TiVo can load.)
int artHeight = Math.min(albumArtView.getHeight(), 480);
int artWidth = Math.min(albumArtView.getWidth(), 640);
albumArtImage = createImage( thisAlbum.getScaledAlbumArt(artHeight, artWidth) );
}
else {
// default_album_art.gif has known dimensions of 300x300, so no scaling is needed
albumArtImage = createImage("default_album_art.gif");
}
albumArtView.setResource(albumArtImage, RSRC_HALIGN_CENTER + RSRC_VALIGN_CENTER + RSRC_IMAGE_BESTFIT);
}
}.start();



The ImageResource objects created in the thread are never remove()'d. Bananas does not perform any resource management. Over time, the HME application will crash with an OOM exception.

[perrce]

The ImageResource objects created in the thread are never remove()'d. Bananas does not perform any resource management. Over time, the HME application will crash with an OOM exception.

But once the screen displaying the resource is displayed, wouldn't the ImageResource be destroyed as well? (Assuming that the screen is the only place the image is displayed.) I'm an old-school C guy, so this new-fangled Java garbage collection is still some pretty deep voo-doo to me.

I'm going to play with everyone's suggestions tomorrow. I'll let you know how it goes. Thanks.

[F8ster]

But once the screen displaying the resource is displayed, wouldn't the ImageResource be destroyed as well?

No, resources are a little funky that way -- resources are cached when created so they're available for future getResource() calls. clearResource() clears them from the view, remove() clears ithem from the TiVo and actually frees the memory. They don't follow the traditional rules for Java object scoping. (At least, I think this is accurate -- doing this from memory and it's been a while.)

-- Dave

[perrce]

resources are cached when created so they're available for future getResource() calls. clearResource() clears them from the view, remove() clears ithem from the TiVo and actually frees the memory. They don't follow the traditional rules for Java object scoping.

Well, no wonder I'm having problems then. I assumed that when the resource is no longer in use that it gets garbage collected. I've got a lot of cleaning up to do in my code. :(

Thanks everyone for the help. I'll let you know if helps the memory situation.

[tvengr]

When a Bananas screen is destroyed, the View hierarchy is removed from the Java application and the HME receiver. The Resources associated with the Views are not removed. Even though a Resource can be attached to a View, the lifetime of a Resource is defined by the application. The primary reason for this is performance. For most applications, screens will share a set of Resources, such as images, fonts, etc. Loading and unloading Resources for every screen transition would slow things down considerably.

The problem is not exactly specific to Java or HME. The Resource objects are not getting garbage collected because the objects are still in use (by the receiver). If the application was written with C & OpenGL and textures were never being released, you would run out of video memory.

If you run Harmonium in the Simulator, open the Resources window and you will see the number of Resources growing as you transition through screens.

[s2kdave]

Both of you are not entirely correct. The java garbage collection rules can't be changed. That albumArtImage resource WILL get (java) garbage collected when the thread is done and the albumArtView gets GC.

He is getting OutOfMemory exceptions in the java app which means that he has a memory problem with his java objects, not the resources on the tivo.

You do however need to clean up your resources when you are finished with them except for resources that are saved in the skin. Because you aren't really ever finished with them until you close the app. Basically the rule of thumb is anything you create, you need equivalent clean up code.

Not calling resource.remove() is not your memory leak problem, it's something else. The Application class uses weak references to hold references to the resources which means it can be GC as long as nothing else is holding onto it.

[perrce]

Ok, so I'm working on resources first. Using the "Show Resource Usage" screen in the simulator, as I understand it, I just need to be concerned about the resources listed under the "Sim" column, right? The "GC" column should be garbage collected eventually? Just making sure I'm looking at the right numbers.

[davidblackledge]

It's very important to note that ImageResource is badly designed in the SDK. When you do createImage(Image) (like when you're using a scaled version of the image), the Resource that's created keeps a reference to the Image object. When you create it, it sends the data to the TiVo immediately, but it keeps the reference for later calls for height and width calculations... it never releases it! The only way to get rid of it is to lose your reference to the image resource.
I tried creating a LightImageResource class that pre-calculates those values then throws away the image reference, but it wouldn't work for me under Galleon - apparently because of tighter permissions there since I had to make the class pretend it was part of the TiVo SDK's package to be a functional subclass.
I also tried creating an Image object wrapper that would dispose of the data as soon as the ImageResource had made the calls it was going to make, but that doesn't work because the image conversion code they use detects the class deep down inside and only supports certain classes.
It helps if your original images are only the minimum size you need, though.
I finally had to give up and just do everything via URL streams which means having your own Factory and figuring out how to stream an Image object (or file from disk).

I've got an application superclass I've been working on that does a bunch of stuff. One of the minor things it does is dealing with an appropriate factory for this stuff (and then secondarily dealing with the URL length limit by allowing you to map a path to a short URL the Factory can unmap)

http://david.blackledge.com /tivo (http://david.blackledge.com/tivo)

[perrce]

I'm looking at the destruction of BScreens again. I've written a ridiculously simple BApp that creates and pushes a BScreen on a KEY_RIGHT and pops the BScreen on a KEY_LEFT. I was playing with it in the simulator while keeping an eye on the Views dialog. When I push a new BScreen, it appears in the Views dialog. When I pop it, the listing for the BScreen does not disappear. The listing for the BScreen gets a "!visible" label, but it is never removed.

Since the BScreen is never removed from the Views dialog, does that mean that the BScreen still exists? If so, how the heck can I get them garbage collected? I'm hoping that I'm misinterpreting the contents of that dialog.

For reference, here is the application I was testing:


package org.dazeend.testApp;

import com.tivo.hme.interfaces.IContext;
import com.tivo.hme.bananas.BApplication;
import com.tivo.hme.bananas.BScreen;

public class TestApp extends BApplication {

@Override
public void init(IContext arg0) throws Exception {
super.init(arg0);
this.push(new BScreen(this), TRANSITION_NONE);
}

@Override
public boolean handleKeyPress(int code, long rawcode) {
switch(code) {
case KEY_RIGHT:
this.push(new BScreen(this), TRANSITION_NONE);
break;
case KEY_LEFT:
this.pop();
break;
}
return super.handleKeyPress(code, rawcode);
}
}

[davidblackledge]

Well, looking at bananas 1.3.1, you're right... Bananas never explicitly removes the BScreen.

What it does do is post a BEvent.ScreenExit to your screen, at which time you can clean up all of your screen's resources (which you can then restore when you receive a BEvent.ScreenEnter... see next paragraph)
I guess the idea is to allow you to reuse the screen reference for a later push, but the application's push/pop stack doesn't keep a reference to it after popping it, so you have to hold on to the reference yourself.
That means you also have to handle removing the Screen yourself after popping away from it.

Unfortunately, "pop" doesn't return the screen reference to make it simple. You have to keep your own reference to the screen somewhere, or save a reference to it during the ScreenExit event. The way "pop" is written, you can NOT safely call "this.remove()" in the ScreenExit event handler because the reverse transition is performed on the old screen AFTER the event is posted.

[s2kdave]

I looked through the code to figure it out. One thing to note is that the Resource Usage that you are looking at represents the resources on the tivo, not the ones in the java heap. Although it is important to clean those up, that isn't your OutOfMemoryException problem.

So it looks like in bananas 1.3.1 they added a finalize method to attempt to clean up the resource when it gets garbage collected. The implementation doesn't look very good, but they obviously realized a problem with it.

davidblackledge is correct in that pop doesn't remove() your screen. It doesn't do that because it doesn't know if you are still going to use it or not so it's your responsibility to do that. Unfortunately they don't make it easy, but I did think of a way. The exit event won't work that david suggested because you get the exit event when you push a new screen on the stack too so you'll end up deleting the screen so when you pop back to it, it won't be there.

You can override BApplication pop(Object)

public void pop(Object arg) {
Object current = getCurrentScreen();
super.pop(arg);
if (current instanceof View) {
//auto remove() upon pop of the current screen being popped
((View)current).remove();
}
}

Now this approach assumes you will never reuse your screens which is sort of wasteful if you don't. You can modify this by firing a custom pop event or method call.


public void pop(Object arg) {
Object current = getCurrentScreen();
super.pop(arg);
if (current instanceof BaseScreen) {
//call the viewPopped() method to let the screen decide
((BaseScreen)current).viewPopped();
}
}

Then in your screen add the viewPopped() method and determine whether to clean it up or not based on that screen instance. You can maybe tie it to a property setting so that the user of the screen can set whether it is "removeOnPop" or not.

[s2kdave]

Basically the HME api is very poorly written and always has been that way. It can easily be rewritten in a solid way given the time to do it. But it works well enough and since there isn't that much demand, it would be kind of a waste of time.