Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Java, lazily initialized field without synchronization

Sometimes when I need lazily initialized field, I use following design pattern.

class DictionaryHolder {
  private volatile Dictionary dict; // some heavy object

  public Dictionary getDictionary() {
    Dictionary d = this.dict;
    if (d == null) {
      d = loadDictionary(); // costy operation
      this.dict = d;
    }
    return d;
  }
}

It looks like Double Checking idion, but not exactly. There is no synchronization and it is possible for loadDictionary method to be called several times.

I use this pattern when the concurrency is pretty low. Also I bear in mind following assumptions when using this pattern:

  • loadDictionary method always returns the same data.
  • loadDictionary method is thread-safe.

My questions:

  1. Is this pattern correct? In other words, is it possible for getDictionary() to return invalid data?
  2. Is it possible to make dict field non-volatile for more efficiency?
  3. Is there any better solution?
like image 652
Andrey Avatar asked Apr 06 '11 14:04

Andrey


3 Answers

I personally feel that the Initialization on demand holder idiom is a good fit for this case. From the wiki:

public class Something {

        private Something() {}

        private static class LazyHolder {
                private static final Something INSTANCE = new Something();
        }

        public static final Something getInstance() {
                return LazyHolder.INSTANCE;
        }
}

Though this might look like a pattern intended purely for singleton control, you can do many more cool things with it. For e.g. the holder class can invoke a method which in turn populates some kind of data.

Also, it seems that in your case if multiple threads queue on the loadDictionary call (which is synchronized), you might end up loading the same thing multiple times.

like image 108
Sanjay T. Sharma Avatar answered Oct 31 '22 18:10

Sanjay T. Sharma


The simplest solution is to rely on the fact that a class is not loaded until it is needed. i.e. it is lazy loaded anyway. This way you can avoid having to do those checks yourself.

public enum Dictionary {
 INSTANCE;

  private Dictionary() {
     // load dictionary
  }
}

There shouldn't be a need to make it any more complex, certainly you won't make it more efficient.

EDIT: If Dictionary need to extend List or Map you can do.

public enum Dictionary implements List<String> { }

OR a better approach is to use a field.

public enum Dictionary {
    INSTANCE;
    public final List<String> list = new ArrayList<String>();
}

OR use a static initialization block

public class Dictionary extends ArrayList<String> {
    public static final Dictionary INSTANCE = new Dictionary();
    private Dictionary() { }
}
like image 28
Peter Lawrey Avatar answered Oct 31 '22 18:10

Peter Lawrey


Your code is correct. To avoid loading more than once, synchronized{} would be nice.

You can remove volatile, if Dictionary is immutable.

private Dictionary dict; // not volatile; assume Dictionary immutable

public Dictionary getDict()
    if(dict==null)
        dict = load()
    return dict;

If we add double checked locking, it's perfect

public Dictionary getDict()
    if(dict==null)
        synchronized(this)
             if(dict==null)
                  dict = load()
    return dict;

Double checked locking works great for immutable objects, without need of volatile.


Unfortunately the above 2 getDict() methods aren't theoretically bullet proof. The weak java memory model will allow some spooky actions - in theory. To be 100% correct by the book, we must add a local variable, which clutters our code:

public Dictionary getDict()
    Dictionary local = dict;
    if(local==null)
        synchronized(this)
             local = dict;
             if(local==null)
                  local = dict = load()
    return local;
like image 3
irreputable Avatar answered Oct 31 '22 18:10

irreputable