3

Is there any functional difference between these two ways of implementing a Singleton?

public class MySingleton {
    private static MySingleton instance;

    public static MySingleton getInstance() {
        if (instance == null) {
            instance = new MySingleton();
        }
        return instance;
    }
}


public class MySingleton {
    private static final MySingleton instance = new MySingleton();

    public static MySingleton getInstance() {
        return instance;
    }
}

Besides the fact that the first way would allow for some sort of clearInstance() method. Though you could just make instance not final in the second method.

Does the first method technically perform better because it is only initialized the first time it is needed instead of when the program starts?

lbenedetto
  • 2,022
  • 1
  • 21
  • 39
  • 1
    The first method is not thread safe. See also ["Double-Checked" Locking is Broken](https://www.cs.umd.edu/~pugh/java/memoryModel/DoubleCheckedLocking.html). I would prefer an enum implementation though. – Elliott Frisch Nov 10 '18 at 18:41
  • Neither of these is singleton because the classes still have default public constructors. – Onik Nov 10 '18 at 19:02
  • Yes, @Onik is correct. You need a private constructor to make sure there won't be additional copies of the Singleton – Francisco Delmar Kurpiel Nov 10 '18 at 19:47

5 Answers5

4

The first one is lazy loading and the second is eager loading. Maybe your application never call the singleton, so if creating new instance of your singleton be heavy resource consuming action, then the lazy loading is better since it create new instance once needed.

Majid
  • 13,853
  • 15
  • 77
  • 113
  • Actually, both method are lazy. Variable initialization and static constructors are involved only when the class is used. In this respect both are equivalent. – Francisco Delmar Kurpiel Nov 10 '18 at 18:58
  • @FranciscoDelmarKurpiel No, Static variables are initialized only once , at the start of the execution . see https://stackoverflow.com/a/8704858/2040375 – Majid Nov 10 '18 at 19:05
  • That answer have a few inaccuracies, as the comments show, including this aspect. Java will initialize only your main class, were main sits, and, as you make references to other classes the class loader will initialize static variables and run static initializers for the classes being referenced. Of course, class initialization is JVM-dependent, but both Oracle and jdk work like this. It's quite easy to test, just put a breakpoint on the Singleton's constructor and write a main *on a different class* that either don't get the Singleton or delays getting it by, say, 10 seconds. – Francisco Delmar Kurpiel Nov 10 '18 at 19:40
  • I did some experimentation, and it turns out I was wrong. At least in the settings I tested the initialization of the static variable happened in parallel with the execution of main. I'll post more details in my answer. – Francisco Delmar Kurpiel Nov 11 '18 at 18:36
3

The first method you use is not thread safe. I would consider it to be a bug.

The second method is simpler, thread safe, fast and, if you make sure the constructor won't throw silly exceptions, correct.

If you absolutely need more logic you can go with the first method, must make sure you protect it with a mutex. Something like:

public class MySingleton {
    private static final Object mylock = new Object();
    private static MySingleton instance;

    public static MySingleton getInstance() {
        synchronized(mylock) {
            if (instance == null) {
                instance = new MySingleton();
            }
            return instance;
        }
    }
}

Clearly the code is more complex, uses more memory, it's slower, you can't declare the variable as final...

Both methods will initialize the Singleton lazily. In Java, all variable initialization and static constructors are involved by the class loader when the class is used, not on the start of the code. If your code path never invokes getInstance the Singleton will never get initialized.

Personally, I avoid singletons, but when I use them is always with an immediate allocation on the variable declaration.

Correction I ran a few experiments, and it turns out class initialization happened in parallel with the execution of the main thread. It didn't waited, as I believed it would. At least on a very simplified test scenario the initialization is eager, but asynchronous.

Francisco Delmar Kurpiel
  • 376
  • 3
  • 14
2

Is there any functional difference between these two ways of implementing a Singleton?

Yes. If you use an initializer in the variable declaration, then the instance is created when the class is initialized, even if the instance is never accessed. If you initialize it in the getInstance() method then the instance is only created if it is accessed. That has thread safety implications. It does does not otherwise make much difference if initializing an instance is cheap and without lasting external side effects, but that may not always be the case.

Does the first method technically perform better because it is only initialized the first time it is needed instead of when the program starts?

If you are going to use an instance in any case then you are going to pay the cost of initializing it at some point no matter what, so there is no performance difference in that sense. However, a thread-safe version of the first method will be slightly more expensive than the second method on the first invocation, and you will pay that extra overhead again on every subsequent invocation.

John Bollinger
  • 160,171
  • 8
  • 81
  • 157
1

Its about Lazy Initialization vs Eager initialization. The difference is, in the first one the instance will not create until you call the getInstance() method, but in the second one its already have been created even before you call the getInstance() method.

Please refer this link if you want more info

Sandeepa
  • 3,457
  • 5
  • 25
  • 41
1

From the unit testing point of view I prefer the lazy instatiatiation. Given that the singleton's initialization has further side effects (which are irrelevant to the actual test), and you want to test a class which needs the singleton (maybe just one particular method), it's easier to mock the singleton and inject it to the instance variable while preparing the test. Using a mock for your singleton instance you have easier control what the singleton's method return to your class under test.

The overhead of the thread safe instantiation can be minimized by the double checked lock pattern:

private static volatile MySingleton instance;

public static MySingleton getInstance() {
    if (instance == null) {
        synchronized ( MySingleton.class ) {
            if (instance == null) {
                instance = new MySingleton();
            }
        }
    }
    return instance;
}

Thus only the rare situation where two (or more) threads access the singleton for the first time (and at the same time) may enter the lock state. Afterwards the first ''if null'' will return false and you never enter the lock state again.

Important: the member has to be declared volatile for this pattern to work reliably.

Note: It has been proven that the above "double checked lock" pattern is not 100 percent reliable. See the discussion below in the comments and especially Brian Goetz' arcticle

Heri
  • 4,368
  • 1
  • 31
  • 51
  • This implementation have a synchronization bug, as the author discloses, and the use of volatile will result in the code generated always reading from RAM, never reusing the value from registers. In my opinion correctness is always more important than a few nanoseconds, even when main go of an app is performance. – Francisco Delmar Kurpiel Nov 10 '18 at 22:37
  • Which sync bug do you mean? I am not aware that my code example should have a bug. – Heri Nov 11 '18 at 10:59
  • Two or more threads may pass the if statement, all of them testing true. The threads will, one at a time create a new instance of the Singleton (that won't be a Singleton anymore), each thread returning a different instance and overriding the stored value on the "instance" member. Also, you are using a publicly available monitor, so you might end up having a deadlock. – Francisco Delmar Kurpiel Nov 11 '18 at 11:28
  • Oops, yes you are right. Forgot to insert the second null check. Corrected my post. – Heri Nov 11 '18 at 11:39
  • I'm sorry, but now you might return partially initialized instances. Just Google "Java double checked lock". This is an anti-pattern and should be avoided. You can make it work by using volatile and an additional local variable, but I ask why you you do it. In my opinion it is **never** worth it. – Francisco Delmar Kurpiel Nov 11 '18 at 12:10
  • For java5 and later this pattern is valid. See https://wiki.sei.cmu.edu/confluence/display/java/LCK10-J.+Use+a+correct+form+of+the+double-checked+locking+idiom. The direct use of the member 'instance' is also valid as long there is no further initialization needed after the constructor (e.g. if you call something like instance.init() after the assignment). – Heri Nov 11 '18 at 12:23
  • Why I use this? See my answer. Injecting a mock directly into the variable in unit tests avoids the call to the constructor entirely. This is well worthy if the constructor does a lot of work (e.g. intializes other things which are not part of the actual test). – Heri Nov 11 '18 at 12:25
  • I'm sorry, I don't want to be confrontive, but that article is wrong on this respect. Please look at https://www.javaworld.com/article/2074979/java-concurrency/double-checked-locking--clever--but-broken.html, on section "Volatile doesn't mean what you think, either". I'll put some code on gitlab to prove the point. – Francisco Delmar Kurpiel Nov 11 '18 at 14:01
  • Ah, thanks for the link. I was not aware about these complications. Learning never ends... ;-) – Heri Nov 11 '18 at 18:38
  • The question is now, shall I delete my post? – Heri Nov 11 '18 at 18:39
  • I believe the conversation we had have value. Personally, I would only add some text to the end of the answer with something you consider appropriate. – Francisco Delmar Kurpiel Nov 11 '18 at 18:55