Use Of Semaphore for avoiding unnecessary downloads/loads

[yevon]

I'm looking into new ways to improve speed significantly. Apart from creating a sized cache version of the image, that i know you're working on, i have new ideas.

I haven't made any test of that one:
Think on a listView full of Images. If they were cached alternatively, one on external disk cache and the other on internal disk cache, the read speed would be greatly improved.

Also there is an issue when you have a list with allways the same url repeated. Imagine that there are 10 image thumbnails on screen.The first time it is downloaded from internet/disk up to 5 times! that is due to there is any request control over the same url. The requests over the same url should be blocked until the image is downloaded to memory/disk cache. This can be achieved using semaphores. I will post some sample code later from the last issue!.

[yevon]

I've made some tests with double cache without any speed improvement, so for the moment ignore it.

For the second idea one way would be like this:

On ImageLoader:

A map that given a url returns a Lock, a method for get a existing lock or new one given a url, and a method that deletes an unused lock with a given url.

private Map<String, ReentrantLock> taskLocks = Collections.synchronizedMap(new WeakHashMap<String, ReentrantLock>());

private ReentrantLock getLock(String uri) {
        ReentrantLock lock = taskLocks.get(uri);
        if (lock == null) {
            lock = new ReentrantLock();
            taskLocks.put(uri, lock);
        }
        return lock;
    }

public void removeLock(String uri) {
        taskLocks.remove(uri);
    }

When calling LoadAndDisplayImageTask we should pass de corresponding lock:

LoadAndDisplayImageTask displayImageTask = new LoadAndDisplayImageTask(configuration, imageLoadingInfo, new Handler(), display, getLock(imageLoadingInfo.uri));

The run method of LoadAndDisplayImageTask would have this structure:

@Override
    public void run() {
        if(lock!=null){
            lock.lock();
        }

        try {
                         //CODE (verifying if the image is in memory cache)
        } finally {
            if(lock!=null){
                lock.unlock();
                if(!lock.hasQueuedThreads()){
                    ImageLoader.getInstance().removeLock(imageLoadingInfo.uri);
                }
            }

        }
    }

Also another point to consider is aborting immediately any operation over images that are not on screen. That happens very often when scrolling. At every stream read/write should be verified that the image is on screen, and if not the case abort it.

[nostra13]

Thanks, yevon, for your work. Appreciate it.
Just one improvement I want to implement: now we can display same image in several ImageView only one after another because of only one task can hold lock at the time.It would be nice to implement next behavior: when image have loaded then all waiting threads can continue their work. I think wait() and notifyAll() should be used. Will see...

[yevon]

With a sempahore / lock you get this behaviour. The idea I gave you doesn't already do that?.
The logic would be:

1 -Get a lock for a corresponding Url. -> return for example googleLock
2 - First of all do googleLock.lock()
3 - Try to get the bitmap from memory. If it is null, it means that the image should be downloaded for first time. It should be probably the first thread that has requested this url.
Any thread that comes after that, will be blocked until the first one has been downloaded and put into memoryCache.
So, after the first thread has done its job, all threads will be wake up simultaneously ( not noticeable). The first thing that they would do, is to verify if the image is in memoryCache. Probably, the image may be in memoryCache (the worst case, it would stay in disk cache), so they have just to post to de UI thread to show it. The UI thread just does one thing simultaneously, so the images would appear one after another but that's an Android limitation (it should be practically instant).
4- Try to get the bitmap from disc Cache
5- Else download bitmap and put it to memory and to disk cache, after that unlock the lock, releasing all waiting threads for THAT url.

Pseudo Code

Lock lock = getLock(String url);
lock.lock();

Bitmap b = getFromMemoryCache();

if(b == null){
      b = getFromDisckCache();
      if(b == null){
             b = getBitmapFromYouWant
             Scale, compress bitmap;
             putToMemoryCache(b);
             putToDiskCache(b);

       }
}
lock.unlock();

postToUiThread(b, ImageView)

Every url has its lock, so you are only blocking all threads that are requesting that url, not other urls. The first one will put it into memory cache, then all others will wake up at the same time getting it from memory cache.

[nostra13]

Maybe I didn't explain clearly what I meant.
My point is several threads can't simultaneously execute code between lock.lock() and lock.unlock().
For example, we have 10 ImageView and try to load image from the same URL into all of them. We call displayImage() 10 times. Thread #1 get lock for URL and other threads are waiting. When Thread #1 complete their work and Bitmap needed for all other thread already is in memory cache then Thread 1 unlocks URI. And then another thread (e.g. Thread #2) get lock and display Bitmap form memory cache, and unlock. Then another thread (#3) can get lock and execute its task. And so on.
But I want that after Thread #1 load image and complete their work threads #2, #3, #4, ... #10 continue their execution simultaneously (for case if we have thread pool >9). Do you understand what I mean?
I'm not sure that it increase displaying speed significantly, but it will, I think.

[yevon]

Ahh yess now I understand you. I don't think that it is noticeable, because getting the image from memory should be practically instant.

mm a solution could be using a sempahore initialized with the size of the thread pool (10 for example).
The first thread should adquire 10 permits and release 10 permits, and other waiting threads adquire 1 permit and release 1 permit.

Mmm i think that this can be done with something like... semaphore.waitingThreads.size() == 0.

if(semaphore.waitingThreads.size() == 0){
sempahore.adquire(10)
}
else{
semaphore.adquire(1);
}

same for release

[nostra13]

Will try it. Thanks.

[nostra13]

Right, difference isn't noticeable.