views:

119

answers:

2

In a nice article with some concurrency tips, an example was optimized to the following lines:

double getBalance() {
    Account acct = verify(name, password);
    synchronized(acct) { return acct.balance; }
}

If I understand that correctly, the point of the synchronization is to ensure that the value of acct.balance that are read by this thread is current and that any pending writes to the fields of the object in acct.balance are also written to main memory.

The example made me think a little: wouldn't it be more efficient to just declare acct.balance (i.e. the field balance of class Account) as volatile? It should be more efficient, save you all the synchronize on accesses to acct.balance and would not lock the whole acct object. Am I missing something?

+5  A: 

You are correct. volatile provides a visibility guarantee. synchronized provides both a visibility guarantee AND serialisation of protected code sections. For VERY simple situations volatile is enough, however it is easy to get into trouble using volatile instead of synchronisation.

If you were to assume that Account has a way of adjusting its balance then volatile is not good enough

public void add(double amount)
{
   balance = balance + amount;
}

Then we have a problem if balance is volatile with no other synchronization. If two threads were to try and call add() together you could have a "missed" update where the following happens

Thread1 - Calls add(100)
Thread2 - Calls add(200)
Thread1 - Read balance (0)
Thread2 - Read balance (0)
Thread1 - Compute new balance (0+100=100)
Thread2 - Compute new balance (0+200=200)
Thread1 - Write balance = 100
Thread2 - Write balance = 200 (WRONG!)

Obviously this is wrong because both threads read the current value and updated independently and then wrote it back (read, compute, write). volatile does not help here so you would need synchronized to ensure one thread completed the entire update before the other thread began.

I general find that if when writing some code I think "can I use volatile instead of synchronized" the answer might well be "yes" but the time/effort of figuring it out for sure and the danger of getting it wrong is not worth the benefit (minor performance).

As an aside a well written Account class would handle all the synch logic internally so callers don't have to worry about it.

Mike Q
+1  A: 

Declaring Account as volatile is subjected to following issues and restrictions

1."Since other threads cannot see local variables, declaring local variables volatile is futile." Moreover If you try to declare a volatile variable in a method, you'll get a compiler error in some cases.

double getBalance() { volatile Account acct = verify(name, password); //Incorrect .. }

  1. Declaring Account as volatile warns the compiler to fetch them fresh each time, rather than caching them in registers. This also inhibits certain optimizations that assume no other thread will change the values unexpectedly.

  2. If you need synchronized to co-ordinate changes to variables from different threads, volatile does not guarantee you atomic access,because accessing a volatile variable never holds a lock, it is not suitable for cases where we want to read-update-write as an atomic operation. Unless you are sure that acct = verify(name, password); is single atomic operation, you cannot guarantee excepted results

  3. If variable acct is an object reference, then chances are it may be null .Attempting to synchronize on a null object will throw a NullPointerException using synchronized. (because you're effectively synchronizing on the reference, not the actual object) Where as volatile does not complain

  4. Instead you could declare a boolean variable as volatile like here

    private volatile boolean someAccountflag;

    public void getBalance() { Account acct; while (!someAccountflag) { acct = verify(name, password); } }

Note you cannot declare someAccountflag as synchronized, as you can't synchronize on a primitive with synchronized, synchronized only works with object variables, where as primitive or object variable may be declared volatile

6.Class final static fields doesn't need to be volatile, JVM takes care of this problem. So the someAccountflag need not be even declared volatile if it is a final static or you could use lazy singleton initialization making Account as a singleton object and declare it as follows: private final static AccountSingleton acc_singleton = new AccountSingleton ();

Sudhakar Kalmari
Thank you for the answer, but I actually suggested not to declare acct as volatile, but acct.balance - that is, the field balance of class Account. That modifies some of your comments. I tried to clarify the question a little in that respect.
hstoerr