Memory Leak in WeakHashMap in SimpleCursorAdapter
TomG
noticed that there is a memory leak occurring due to the WeakHashMap
mHolders there. In this mapping, the key is set as a view and the
values are the children of that view. The children hold a strong
reference to their parent, the view. Therefore, this causes the
WeakHashMap to not be able to remove items despite no outside
references to these views, due to the circular reference in the
mapping. Normally, this does not show up as much of a problem as the
number of views added to the map will stop growing at a point once
they are all inflated. However, I am working on some code which is
grabbing views via getView for some other purposes, which quickly uses
up my memory, as all the views I inflate get put in the mapping and
never get released (until the adapter is cleaned up). I have resolved
the issue for now by removing the weakhashmap as it did not seem to
serve much purpose (I simply grab the information I need directly in
bindView rather than storing it in mHolders beforehand). I would like
to know if anyone has any ideas on what the performance impact of
removing the weakhashmap would be. It appears to be extremely minimal,
but I assume the hashmap was in there for some reason. Additionally,
if anyone has a better solution for this leak, that would be
appreciated.
Thank you.
Romain Guy
> --
>
> You received this message because you are subscribed to the Google Groups "android-platform" group.
> To post to this group, send email to android-...@googlegroups.com.
> To unsubscribe from this group, send email to android-platfo...@googlegroups.com.
> For more options, visit this group at http://groups.google.com/group/android-platform?hl=en.
>
>
>
--
Romain Guy
Android framework engineer
roma...@android.com
Note: please don't send private questions to me, as I don't have time
to provide private support. All such questions should be posted on
public forums, where I and others can see and answer them
TomG
Thank you for the response. Do you feel removing the hashmap is an
acceptable solution? Do you see any negative implications to this
approach?
> > For more options, visit this group athttp://groups.google.com/group/android-platform?hl=en.
>
> --
> Romain Guy
> Android framework engineer
Romain Guy
the adapter to do the findViewById() every time getView() is called.
> For more options, visit this group at http://groups.google.com/group/android-platform?hl=en.
>
>
>
--
Romain Guy
Android framework engineer
TomG
Is there an open ticket for this on gerrit? If so, can you please
attach a link to it?
Thank you.
Romain Guy
there's no bug on the public tracker.
> For more options, visit this group at http://groups.google.com/group/android-platform?hl=en.
>
>
>
--
Romain Guy
Android framework engineer
Brad Larson
I think Tom and I would be glad to try and code up a fix, test it out,
and submit it to aosp, but we might need a nudge in the right
direction. If removing the hashmap will harm performance, are there
any alternatives to keep performance but avoid the memory leak? How
would you like to see this bug resolved?
Thanks!
Brad
TomG
Are there some specific cases where you think we would see a large
decrease in efficiency by removing the weakhashmap? I have done some
analysis of the costs of removing the weakhashmap and am finding there
to be no significant efficiency differences between retrieving the
views from the map and for traversing the child views inside bindView.
I will explain my testing below.
I have a list which utilizes a SimpleCursorAdapter. This list contains
187 TextViews. I scrolled down and then back up through this list to
inflate the views for the various items in the list and took timings
for the difference between the implementations. Firstly, we can ignore
all cases where we are creating a new view, as in those cases,
removing the weakhashmap will clearly provide a performance increase
as we have to do the traversing of children views in this situation
anyway and, in the case of the current implementation, pull the views
back out of the weakhashmap as well. Therefore, we look at where we
have a recycled view to use.
The difference between the weakhashmap implementation and without is
that the cost in bindView is pulling from the weakhashmap and
traversing the children views, respectively. Therefore, I timed the
call to pull from the weakhashmap in the original implementation and
timed in the new implementation everything related to the traversal
(basically the time we pay for generateViewHolder in the original
implementation). Scrolling through the list and watching these costs
when we have a convertview, I find the following average cost per call
to bindView:
Original Implementation: 18500 nanoseconds
Weakhashmap Removed: 18000 nanoseconds
Multiple runs show slightly varying results, but the average time with
the weakhashmap removed is lower, or at least equivalent to the
original. If we had several children views in the item, we would
increase the cost when the weakhashmap is removed more than with the
weakhashmap, but only by a few thousand nanoseconds, an insignificant
amount even if we were binding hundreds upon hundreds of views in a
list. Additionally, this does not take into account the cost of the
management of the weakhashmap or whatever storage class we would
implement as its replacement for the storage of these views, not to
mention the temporary memory cost of storage in the container.
Please let me know if there are problematic cases I am not seeing and
any thoughts on my analysis.
Thank you,
Tom
Romain Guy
frames per second while flinging a list. It all depends on the
complexity of the list items.
> For more options, visit this group at http://groups.google.com/group/android-platform?hl=en.
>
>
>
--
Romain Guy
Android framework engineer
TomG
If that is the case, do you have a solution in mind for this issue, or
at least the beginning of one so I can explore a proper fix for this
issue?
Romain Guy
> For more options, visit this group at http://groups.google.com/group/android-platform?hl=en.
>
>
>
--
Romain Guy
Android framework engineer
Romain Guy
should be removed. I can't think of another safe way to do this. The
other Simple*Adapters are also suffering from the same issue. I'll
have to make that change internally too because I don't know when the
next merge is going to be.
On Wed, Dec 30, 2009 at 1:53 PM, TomG <twgr...@gmail.com> wrote:
> For more options, visit this group at http://groups.google.com/group/android-platform?hl=en.
>
>
>
--
Romain Guy
Android framework engineer