Java synchronized counter - what about get()?

Question

It's known that simple x++ is not an atomic operation, but in fact is read-increment-write operation. That is why it should be synchronized. But what about get()? I have read it should be synchronized too, but could someone explain me why? To avoid memory consistency errors by introducing happens-before relationship? What about a situation when get() is called very often by multiple threads and the value is rarely changed. Isn't synchronized get() slowing them down? Are there any other ways to achieve synchronization in that scenario (not using AtomicInteger)? Would volatile keyword work here?

public class Counter {
    private int value;

    public synchronized int get() { return value; }
    public synchronized int increment() { return ++value; }
    public synchronized int decrement() { return --value; }
}

Thanks!

edit:

I would like to make myself clear. By using volatile I meant introducing that keyword and removing synchronized in get() method. I was wondering if it will make it thread-safe but also more efficient if many threads are reading the value and one is rarely changing it.

Answer 1

Are there any other ways to achieve synchronization in that scenario (not using AtomicInteger)?

First off, you should be using AtomicInteger if you can. I'm not sure why you would not use it.

Would volatile keyword work here?

Yes except not for the ++. AtomicInteger provides a safe increment without locking. If you want to roll your own (for some crazy reason) then you will need the blocking or you need to duplicate the AtomicInteger internal spin mechanisms.

But what about get()? I have read it should be synchronized too, but could someone explain me why?

AtomicInteger wraps a volatile int to provide its functionality. When you access the volatile field you cross a memory barrier on the get() as well. You need to cross that memory barrier to ensure that if another thread has updated the value, the thread calling the get() sees the update. Otherwise, a thread could be working with a outdated value.

Answer 2

If you repeatedly read a non-volatile value, the JVM can cache the value in a register can you might not see any changes.

The way to avoid this is to make the value volatile.

However if performance is a concern, use AtomicInteger which is lock-less.