static vs non-static inner classes

I have been finding it convenient to extend Handler in many of my
activities to handle messages specific to the activity. The handler
sublass is an inner class of the activity and needs to access its
state. I am wondering if there is any performance difference between
making the handler subclass static and passing in the activity
explicitly in its constructor or making the subclass an "instance
class" and letting the vm worry about my accessing members of the
containing activity.

I read somewhere that non-static inner classes are to be avoided
because of the implicit reference to the containing class instance
that needs to be associated with the inner class. But in the static
example above I am just providing this reference explicitly. Also it
seems like non-static inner classes are used liberally in the Android
source code and examples. So maybe my fear of non-static inner classes
is unfounded?

(I have read the Designing for Performance best-practice doc and taken
its advice of using package scope with inner classes to heart. But I
don't think it quite answers my question -- all the more reason for
thinking that the distinction between static and non-static inner
classes is of no real significance?)

On Mar 9, 7:17 pm, Greg Krimer <gkri...@gmail.com> wrote:
> I have been finding it convenient to extend Handler in many of my
> activities to handle messages specific to the activity. The handler
> sublass is an inner class of the activity and needs to access its
> state. I am wondering if there is any performance difference between
> making the handler subclass static and passing in the activity
> explicitly in its constructor or making the subclass an "instance
> class" and letting the vm worry about my accessing members of the
> containing activity.

There's no real difference between explicitly and implicitly passing
the outer-class reference around. Your example above is essentially
doing what javac does automatically.

Static inner classes have some useful properties, e.g. you know
they're not modifying state in the parent, and you can use
Class.newInstance() with them. There's no performance magic though.

If you're really curious, write it both ways in trivial source files,
compile them, then view them with "javap -private -verbose <Class>" or
"dx --dump <Class.class>". They should look about the same.

Thank you very much for the reply! After reading what Fadden said
weeks ago, I happily converted my static inner handler classes to non-
static ones.

Just recently, I was browsing the source code of Handler and came
across this bit of internal Android debugging:

/*
* Set this flag to true to detect anonymous, local or member
classes
* that extend this Handler class and that are not static. These
kind
* of classes can potentially create leaks.
*/
private static final boolean FIND_POTENTIAL_LEAKS = false;

There is some code further down that uses reflection to locate such
subclasses if this flag is true.

My handlers fall into the category of non-static member classes, so I
am curious how such classes could cause a memory leak. My initial
thinking was that handlers could perhaps accumulate in a collection
associated with the MessageQueue. But handlers are never placed into
collections, as far as I can tell. The reference to the handler is in
the message. Clever! (And in any case if the memory leak is due to an
ever-growing collection of handlers somewhere then that has nothing to
do with whether or not the handler class is static or not.)

Just wondering if someone can shed some light on this comment in the
Android source code.

Thanks,

Greg

On Mar 10, 4:08 pm, fadden <fad...@android.com> wrote:
> On Mar 9, 7:17 pm, Greg Krimer <gkri...@gmail.com> wrote:
>
> > I have been finding it convenient to extend Handler in many of my
> > activities to handle messages specific to the activity. The handler
> > sublass is an inner class of the activity and needs to access its
> > state. I am wondering if there is any performance difference between

> > making the handler subclassstaticand passing in the activity

> > explicitly in its constructor or making the subclass an "instance
> > class" and letting the vm worry about my accessing members of the
> > containing activity.
>
> There's no real difference between explicitly and implicitly passing
> the outer-class reference around. Your example above is essentially
> doing what javac does automatically.
>

I wrote that debugging code because of a couple of memory leaks I
found in the Android codebase. Like you said, a Message has a
reference to the Handler which, when it's inner and non-static, has a
reference to the outer this (an Activity for instance.) If the Message
lives in the queue for a long time, which happens fairly easily when
posting a delayed message for instance, you keep a reference to the
Activity and "leak" all the views and resources. It gets even worse
when you obtain a Message and don't post it right away but keep it
somewhere (for instance in a static structure) for later use.

If you want to use the equivalent of an inner-class but not risk a
leak, here's a simple pattern:

> I have been finding it convenient to extend Handler in many of my
> activities to handle messages specific to the activity. The handler
> sublass is an inner class of the activity and needs to access its
> state. I am wondering if there is any performance difference between
> making the handler subclass static and passing in the activity
> explicitly in its constructor or making the subclass an "instance
> class" and letting the vm worry about my accessing members of the
> containing activity.

There's no real difference between explicitly and implicitly passing
the outer-class reference around. Your example above is essentially
doing what javac does automatically.

A non-static inner class, nested in an Activity, can be a bad thing if it's somehow carried over the activity being recreated (i.e. configuration change), causing GC for the old activity instance to be delayed until it (the inner class) goes away, presumably with the new Activity instance. On a platform with limited memory resources this can be undesirable.

I recall seeing some code on this list that did this with AsyncTask, there may be other cases.

I've implemented a Handler.Callback wrapper that wraps the Callback (usually an inner class) in a WeakReference, and I construct the Handler instance using the wrapped Callback. This way, the code that used to exist in the leaky Handler inner class now exists in the Callback implementation. This seems to have the least impact on the source code, while still removing the potential leak.

But what about a Handler.Callback interface which can be passed to the Handler's constructor. According to the source code it is stored in the Handler class as a strong reference, so if the Message lives in the queue for a long time, the callback will not be gc'ed. And the callback may be an Activity or an inner class of an activity. But I can't see any checks in the source code and any recommendations in the documentation. As far as I understand, there isn't any right way to use the Handler.Callback with the Handler, is there?

On Saturday, April 4, 2009 9:52:06 PM UTC+4, Romain Guy wrote:

I wrote that debugging code because of a couple of memory leaks I
found in the Android codebase. Like you said, a Message has a
reference to the Handler which, when it's inner and non-static, has a
reference to the outer this (an Activity for instance.) If the Message
lives in the queue for a long time, which happens fairly easily when
posting a delayed message for instance, you keep a reference to the
Activity and "leak" all the views and resources. It gets even worse
when you obtain a Message and don't post it right away but keep it
somewhere (for instance in a static structure) for later use.

If you want to use the equivalent of an inner-class but not risk a
leak, here's a simple pattern:

>> On Mar 9, 7:17 pm, Greg Krimer <gkri...@gmail.com> wrote:
>>
>> > I have been finding it convenient to extend Handler in many of my
>> > activities to handle messages specific to the activity. The handler
>> > sublass is an inner class of the activity and needs to access its
>> > state. I am wondering if there is any performance difference between

>> > making the handler subclassstaticand passing in the activity

>> > explicitly in its constructor or making the subclass an "instance
>> > class" and letting the vm worry about my accessing members of the
>> > containing activity.
>>
>> There's no real difference between explicitly and implicitly passing
>> the outer-class reference around. Your example above is essentially
>> doing what javac does automatically.
>>

>> Staticinner classes have some useful properties, e.g. you know

>> they're not modifying state in the parent, and you can use
>> Class.newInstance() with them. There's no performance magic though.
>>

>> If you're really curious, write it both ways in trivial source files,
>> compile them, then view them with "javap -private -verbose <Class>" or
>> "dx --dump <Class.class>". They should look about the same.
> >
>

It's also pretty easy to remove outstanding messages in an activity or fragment's end-of-lifecycle callbacks. Good idea anyway if those messages are supposed to trigger UI changes -- you won't want them to arrive after the activity/fragment has shut down.

The problem is with a non-static inner class is that you don't 'see' the implicit reference to the outer instance (OuterClass.this). It is easy to forget about this one and later realize what problems it causes.

Usually,

when the lifetime cycle of an inner class matches the lifecycle of the outer class (e.g. OnClickListeners,etc, or BroadCastReceivers that are unregistered in the (outer) Activity's onDestroy), I make it non-static.

When the lifetime cycle of an inner class does not match the lifecycle of the outer class, I make it static. If a reference to the outer class' instance is needed, I either 'wrap' it in a WeakReference or create specific methods for detaching the outer instance from the inner instance.