proper usage of synchronized singleton?

Question

So I am thinking about building a hobby project, one off kind of thing, just to brush up on my programming/design.

It's basically a multi threaded web spider, updating the same data structure object->int.

So it is definitely overkill to use a database for this, and the only thing I could think of is a thread-safe singleton used to contain my data structure. http://web.archive.org/web/20121106190537/http://www.ibm.com/developerworks/java/library/j-dcl/index.html

Is there a different approach I should look in to?

Solution 1

Double-checked locking has been proven to be incorrect and flawed (as least in Java). Do a search or look at Wikipedia's entry for the exact reason.

First and foremost is program correctness. If your code is not thread-safe (in a multi-threaded environment) then it's broken. Correctness comes first before performance optimization.

To be correct you'll have to synchronize the whole getInstance method

public static synchronized Singleton getInstance() {
   if (instance==null) ...
}

or statically initialize it

private static final Singleton INSTANCE = new Singleton();

Solution 2

Using lazy initialization for the database in a web crawler is probably not worthwhile. Lazy initialization adds complexity and an ongoing speed hit. One case where it is justified is when there is a good chance the data will never be needed. Also, in an interactive application, it can be used to reduce startup time and give the illusion of speed.

For a non-interactive application like a web-crawler, which will surely need its database to exist right away, lazy initialization is a poor fit.

On the other hand, a web-crawler is easily parallelizable, and will benefit greatly from being multi-threaded. Using it as an exercise to master the java.util.concurrent library would be extremely worthwhile. Specifically, look at ConcurrentHashMap and ConcurrentSkipListMap, which will allow multiple threads to read and update a shared map.

When you get rid of lazy initialization, the simplest Singleton pattern is something like this:

class Singleton {

  static final Singleton INSTANCE = new Singleton();

  private Singleton() { }

  ...

}

The keyword final is the key here. Even if you provide a static "getter" for the singleton rather than allowing direct field access, making the singleton final helps to ensure correctness and allows more aggressive optimization by the JIT compiler.

Solution 3

If your life depended on a few microseconds then I would advise you to optimize your resource locking to where it actually mattered.

But in this case the keyword here is hobby project!

Which means that if you synchronized the entire getInstance() method you will be fine in 99.9% of all cases. I would NOT recommend doing it any other way.

Later, if you prove by means of profiling that the getInstance() synchronization is the bottleneck of your project, then you can move on and optimize the concurrency. But I really doubt it will cause you trouble.

Jeach!

Solution 4

Try the Bill Pugh solution of initialization on demand holder idiom. The solution is the most portable across different Java compilers and virtual machines. The solution is thread-safe without requiring special language constructs (i.e. volatile and/or synchronized).

http://en.wikipedia.org/wiki/Singleton_pattern#The_solution_of_Bill_Pugh

Solution 5

as Joshua Bloch argues in his book "effective java 2nd edition" I also agree that a single element enum type is the best way to implement a singleton.

public enum Singleton {
  INSTANCE;

  public void doSomething() { ... }
}

View more solutions

Comments

[brady]

I based my answer on the fact that the article cited focuses on the use of double-checked locking to initialize the singleton lazily. This article would be a very bad guide to follow.

[matt b]

Ah, yes thats correct. Double-checked locking is broken anyway.

[matt b]

this pattern is known as a "static initializer"

[Dan]

This is only true if the collection is exposed outside of the singleton. If it is not, then there is no reason to worry about it.

[Bob Cross]

Yes, I was trying to stick with the original vocabulary, though.

[Dan.StackOverflow]

Yes, this is probably the way to do it. But now I am thinking why not make a static member ConcurrentHashMap in my threaded spider class? I guess really my question is, this is one way to do it, is there a better way?

[Bob Cross]

A HashMap is very compelling. It's simple, fast, comes with the JDK, simple and simple ;-). For more specific guidance, I think we need more information on exactly what it is that you want to do (and that would likely be outside the scope of this question).

[tvanfosson]

@Jeach -- not true. Either the collection needs to be synchronized or the methods that you expose that interact with it need to be synchronized. Precisely because you have a single instance, you need to take care to make sure only one thread is in a critical section (like updating the count related to an object in this case) at a time. It's much easier to just use a synchronized collection than to implement all the locking code in your methods.

[Tim Frey]

On line 3, I think you mean Singleton.class. Your example might be clearer if you rename the reference from 'singleton' to 'instance'.

[mamboking]

You are correct. Why don't these edit boxes have code completion :)

[Steve Kuo]

The double-checked lock pattern has been proven to be broken.

[Andy Dennie]

As I understand it, that technique specifically applies to Serializable singletons, and the OP didn't mention anything about that. Still, for future readers, it's worth noting here.

[M4rk]

How can I solve PMD error "Use block level rather than method level synchronization"?

[Murillo Ferreira]

Would be possible to use static blocks for instance variable initialization instead of using syncrhonized method? Static blocks will run at the first time class is loaded by jvm, would this be thread-safe?