Best place for the LruCache? & What about Bitmap.recycle()?


#1

[size=150][color=#0040FF]The Challenge[/color][/size]
Does it make sense to use multi-threading and queue a message when the Bitmap is already in the cache? I am thinking the job should be done in GalleryItemAdapter.getView(). At that point you check the cache and then either set the ImageView’s ImageResource if the Bitmap is in the cache, or put the default pic of Brian in it and create the task to download it. It would be more efficient, and I don’t see how having the downloader class do the caching makes sense from an OOP perspective.

Also, wouldn’t it make sense to modify GalleryItem to also have its ImageView? It would create it in the constructor and do the setImageResource() there. The class would no longer be a “Model” one. Another option is to create a new class that has the GalleryItem and the ImageView.

[size=150][color=#0040FF]Bitmap.recycle()[/color][/size]
I am also wondering about why we don’t want to do [color=#BF0000]Bitmap.recycle()'s[/color] for this project. It is not important when dealing with numerous small Bitmaps? I tried to add code to do it but ended up getting exceptions when it recycled the Brian pic and then tried to use it again.

[size=150][color=#0040FF]Generics[/color][/size]
And, while I am at it, I think the use of generics with the BitmapDownloader class is wrong. Only the View object is generic, not the object (Bitmap) being downloaded. So, it can only be used to download Bitmaps. What other View class would you possibly use?

Just to see if I can do it and learn more about generics, I decided to make an abstract class that makes both generic:

It has abstract methods to do the object specific work:

abstract ObjToReceive getObjectToReceive(byte[] objBytes); abstract byte[] getUrlBytes(String url) throws IOException;

And the implementing class just needs to implement those two methods:

[code]@SuppressWarnings(“hiding”)
public class BitmapDownloader<ImageView, Bitmap> extends InternetObjectDownloader<ImageView, Bitmap> {
public BitmapDownloader(Handler responseHandler, int messageNo, String name) {
super(responseHandler, messageNo, name);
}

@SuppressWarnings("unchecked")
@Override
Bitmap getObjectToReceive(byte[] objBytes) {
	// TODO Auto-generated method stub
	return (Bitmap) BitmapFactory.decodeByteArray(objBytes, 0, objBytes.length);
}

@Override
byte[] getUrlBytes(String url) throws IOException {
	return new FlickrFetchr().getUrlBytes(url);
}

}[/code]

It works, but I don’t know why I needed that @Suppress annotation. It says that “Bitmap is hidden by Bitmap”?? :open_mouth:

Thanks.


#2

You’ve got a lot here to unpack, Rick. Let’s see what I can help out with.

1- Does it make sense to queue a message to hit the cache?

This is a design question with no hard and fast answer. My preference would be to implement it so that a cache hit doesn’t require a message to be sent.

2- I think giving GalleryItem an ImageView reference is not a good idea, for exactly the reason you state - GalleryItem is a Model object. Attaching an ImageView to it makes it difficult to reuse that object in other contexts cleanly, or to pass a GalleryItem around your app.

Your latter idea (of having a class that packages up a GalleryItem with its associated ImageView) sounds like a better way to achieve what you’re thinking of.

3- You’re right - it’s just not an issue when dealing with smaller bitmaps with a high churn rate. It’s large bitmaps that cause major headaches.

4- Generics are a power tool. Once you learn to use them, it’s extremely tempting to make EVERYTHING as generic as possible.

I recommend not doing this - at least not immediately. Every time you make something more generic, the harder it is to understand what specifically it is supposed to do. So try not to make something generic unless there’s a compelling reason to do so.

I originally wrote BitmapDownloader as a non-generic class. We decided that this was unwise - nothing about downloading bitmaps really requires a tight coupling to the idea of an ImageView. A controller might want to download bitmaps for all kinds of reasons apart from putting them in an ImageView. So we rewrote BitmapDownloader as a generic bitmap downloading tool. This requires the controller to write a little bit more code, but yields a huge benefit - the downloader can be used for any view under the sun.

Of course, you could abstract away the idea of downloading bitmaps, too, as you have. And then you end up with something not unlike the Volley library, which abstracts away the idea of downloading “stuff” and creating something useful out of that stuff to be processed on the main thread.

And what is one of the first things that folks end up actually using in Volley? A concrete implementation that downloads and parses Bitmaps, and sets them up on ImageViews…

Anyway, the rabbit hole goes deep. And wide.

5- That unchecked warning is likely just you running into one of the many limitations of Java generics. Sorry. :frowning:


#3

Thanks!! I can’t get recycle() to work. With an API 8 device, when I page down and back up really fast to cause the LruCache to evict items and then need them again I get an exception that it is trying to use a recycled bitmap. Adding checks to see if the bitmap is recycled does not help.

All accesses to the LruCache are on the main thread. The GetView() method uses the bitmap from the cache if it is there. The onObjectDownloaded() method is where it gets added. I override the LruCache.entryRemoved() method to recycle it. So, somewhere after getView() tries to re-use it the LruCache is evicting it.

However, I see no differences in available memory when it recycles it or not. in getView() I added this to check every 25 calls:

callCount++; if (callCount > 25) { callCount = 0; final int freeMemory = (int) (Runtime.getRuntime().totalMemory() / 1024); Log.e(TAG, "Memory Check " + freeMemory + "K available"); }

At least on the emulator, Android does a good job of garbage collection anyway.


#4

You should make sure that you will have no references to a bitmap after you call recycle() on it. This requires you to know who owns the bitmap at any given time, which isn’t a lot of fun - especially in your situation, with caching involved.

Calling recycle() is important mostly for 2.3 and previous devices. Even there, you may not see the effects in totalMemory(), as it may count only the Java heap.


#5

Can I please see your solution to the challenges?

The read-ahead challenge is quite challenging:

  1. When onThumbnailDownloaded() processes a non-visible item it and sets the ImageView to the Bitmap, GalleryItemAdapter.getView(_) gets called with [color=#BF0000]position = 0[/color]. So, when reading ahead 30 items, I was seeing 30 such calls. This causes one of the items currently displayed to quickly display a different Bitmap as those 30 items pass through getView(). No clue why one of the visible items does this when position 0 isn’t in the view.

Setting the item’s ImageView to the Bitmap is necessary for the visible items to display it. If instead you just add the Bitmap to the LruCache and call notifyDataSetChanged(), getView() gets called for every item in the display every time one of them is updated.

So, I added flags to GalleryItem:

[color=#0000FF]private boolean mQueued;[/color] - Prevent an item from being queued when it is already queued (like an item becomes visible while the read-ahead is still processing).
[color=#0000FF]private boolean mNotify; [/color]- For non-visible items (being read ahead) this is set on the 30th item so onThumbnailDownloaded() calls notifyDataSetChanged() only on the 30th item.
So, items 1-29 just get added to the Lru, and the one call to notifyDataSetChanged() causes getView() to get called once for each currently visible item.
[color=#0000FF]private boolean mVisible;[/color] - For visible items call item.getImageView().setImageBitmap(thumbnail);

If I scroll down quickly, bringing read-ahead items into view before they have finished downloading, the app bogs down, and it takes a while for them to get displayed.

  1. When the read-ahead items call onThumbnailDownloaded() and get added to the LruCache, the LruCache evicts older items. So I see 30 such evictions take place, and I see numerous LogCat messages about GarbageCollection tasks running, which causes the app to stall. (and I see no change in those GC messages if I do a recycle() on each bitmap).

I haven’t tried to fix this one. The read-ahead number is visible count X 2. Making it less or more does not work well.

With all that, the app works as smoothly as possible. The calls to getView() are limited. The major hic-up being if you scroll down really fast.


#6

The purpose of the challenges is to think through a harder problem independently, so we don’t provide solutions for them. For the same reason, I generally provide only hints to challenge solutions here in the forums.

This is probably the deepest challenge in the book. As you can tell, you can iterate a lot getting the behavior to be absolutely perfect.

One suggestion: it’s probably better to only prefetch when the list is scrolling, not when it is idle.


#7

I appreciate that, but having thought through it and coming up with the best solution I can muster, I still wonder if there is a better way to do it.

I have the onScroll() method do all the work for determining when the app should read the next page and/or the next 30 items. Since that function includes firstVisibleItem and visibleItemCount, it is the ideal place to calculate the number of read-ahead items. Also, if it gets called before the user scrolls and can read ahead the next 30 items before the user scrolls down to see them.

What I feel iffy about is how to manage the GridView’s ArrayList in the onThumbnailDownloaded() method. It seems the only things I can do there are add the Bitmap to the ImageView or call notifyDataSetChanged().

Here is what it looks like:

mThumbnailThread.setListener(new BitmapDownloader.Listener<GalleryItem, Bitmap>() { public void onObjectDownloaded(GalleryItem item, Bitmap thumbnail) { if (item != null) { item.setQueued(false); if (thumbnail == null || thumbnail.getHeight() == 0) { mItems.remove(item); ((GalleryItemAdapter) mGridView.getAdapter()).notifyDataSetChanged(); Log.w(TAG, "onObjectDownloaded() NULL Bitmap " + item.getId()); return; } if (isVisible()) { if (item.isVisible()) { item.getImageView().setImageBitmap(thumbnail); } else if (item.isNotify()) { // Set for the last read-ahead item ((GalleryItemAdapter) mGridView.getAdapter()).notifyDataSetChanged(); } mGalleryCache.put(item.getIdNo(), thumbnail); } } } });

However, doing the next chapter on Search, brought up the possibility that the list of photos you get back may not be valid. I added code to FlickrFetchr.parseItems() to not add it to the list if the url or id is null. I set the download count to 200, and am finding I may bet back around 175 valid items with that. A null URL caused problems, and I parse the ID to a long to use it as the key in the LruCache.

I also now parse the element for the “pages=” attribute for controlling the page read-aheads. When it sees that the last requested page equals this number, it sets a variable that PhotoGalleryFragment can check to see if there are more pages to read.

Obviously, I know there is never just one way to do everything, but I wonder if there is something in the API that would make this easier and better.

Thanks.