Java cached object return old value

Question

I have Cached List, my code looks like this

public class MyList {
private final List<String> cache = new ArrayList<String>();

private List<String> loadMyList() {
    // HEAVY OPERATION TO LOAD DATA
}

public List<String> list() {
    synchronized (cache) {
        if( cache.size() == 0 ) {
            cache.addAll(loadMyList());
        }
        return Collections.unmodifiableList(cache);
    }
}

public void invalidateCache() {
    synchronized (cache) {
        cache.clear();
    }
}
}

Because list load is very heavy I got a request that if list load is currently in progress i return "old" cached data...

Is that possible, and can someone give me pointers on how to proceed from here

EDIT : Adam Horvath And bayou.io sugested something like this

public class MyList 
{
private final List<String> cache = new ArrayList<String>();
private final List<String> oldCache = new ArrayList<String>();
private volatile boolean loadInProgress = false;

private List<String> loadMyList()
{
    // HEAVY OPERATION TO LOAD DATA
}

public List<String> list()
{
    synchronized (cache)
    {
        if( loadInProgress )
            return Collections.unmodifiableList( oldCache );
        else
            return Collections.unmodifiableList(cache);
    }
}

public void invalidateCache()
{
    synchronized (cache)
    {
        // copy to old cache
        oldCache = new ArrayList<String>( cache );
        // set flag that load is in progress
        loadInProgress = true;
        // clear cache
        cache.clear();

        // initialize load in new thread
        Thread t = new Thread(new Runnable()
        {
            public void run()
            {
                cache.addAll( loadMyList() );
                // set flag that load is finished
                loadInProgress = false;
            }
       });  
       t.start();


    }

   }
}

Will this edited code have any issues?? Since I am not experienced in multithreading and/or cashing optimisation i would appreciate any and all performance suggestion

Virus

If I am not mistaken you may be looking for CopyOnWriteArrayList – Psalmbook

I assume you mean "cached" not "cashed" – Chosen

In your example, people calling list() get a view on your list - that means that when you clear / repopulate the list, callers that still have a copy of that view could see the list in an unstable state. Are you sure you want that? Also there is no "old" data in your example - either the list is empty or it's got something but once it is loaded it is not updated again... – Carbohydrate

What is the problem with this implementation you are trying to solve? – Chosen

@PeterLawrey At 'invalidateCache()' i will call 'cache.addAll(loadMyList())' but i want all threads that call 'list()' in that time to get old cashed data untill 'loadMyList()' is finished, instead of waiting – Virus

@Carbohydrate yes my example is curent state in wich there is no 'Old' data, simply because i don't know where to start, i can add 'new ArrayList<String>' and copy data to it before clearing but i don't know how to return that data if loading is in progress, and i don't know if even that method is the way to go – Virus

no, copy-on-write is not a solution here. I'm surprised that there's no answer yet. people are not as motivated as before on SO.... – Spue

if the 1st reader after invalidation is responsible to load the data, it'll block the reader's thread. that is probably undesirable; we want all reader to return quickly. therefore it's better that invalidation immediately triggers loading, possibly in a new thread. if a new invalidation arrives while the loading is still ongoing, we must arrange another loading, and make sure the 2nd result is cached later. – Spue

@bayou.io I edited my code, did you had something like that in mind?? – Virus

Answer 1

"Because list load is very heavy I got a request that if list load is currently in progress i return "old" cached data..."

This will not happen because of your "synchronized (cache)" block. You need a volatile boolean flag (mutex) to tell you a List is being generated. When a Thread tries to get list() and mutex is true, then it will receive the Cached one. Set it false when loadMyList() is completed.

So, remove the synchronized block and start loading your list in a seperate Thread.

public class MyList {
    private List<String> cache = new ArrayList<String>();
    private volatile boolean loadInProgress = false;

    private List<String> loadMyList() {
        // HEAVY OPERATION TO LOAD DATA
    }

    public List<String> list() {
        // Whatever is in cache, you can always return it
        return Collections.unmodifiableList(cache);
    }

    /**
     * Starts the loader-thread and then continues.
     */
    public void invalidateCache() {
        // Next two lines make sure only one Loader-thread can be started at the same time
        synchronized (cache) {
            if (!loadInProgress) {
                // initialize load in new thread
                Thread t = new Thread("Loader-thread") {
                    @Override
                    public void run() {
                        List<String> toAssign = loadMyList();
                        // You can simply assign instead of copying
                        cache = toAssign;
                        // cache now "points to" refreshed list
                        loadInProgress = false;
                    }
                };
                loadInProgress = true;
                t.start();
                // Now let's exit the synchronized block. Hopefully the Thread will start working soon
            } else {
                // A Thread is already working or about to start working, don't bother him
            }
        }
    }
}

Understructure

A separate status flag seems like a bad design choice. There are plenty of options where you don't need this. (See Pshemos comment above for example) – Hindward

@Hindward Good idea! But I've felt like he could learn about threading by fixing his implementation. – Understructure

@AdamHorvath I edited my question, did you had something like that in mind?? – Virus

This version has non-blocking methods: list() immediately returns with "something" and invalidateCache() will not halt it's invoker-thread with the long list-building operation. Also makes sure only one thread can execute the loadMyList() at the same time. – Understructure