CODEWITHSUNDEEP
javaconcurrencysynchronizationjava.util.concurrent
gbtimmon
gbtimmon

Reputation: 4342

Do synchronized code blocks only block for assignments or for the entire block body?

I have a data var, monthArray, that is read by multiple consumers and periodically updataed by a single periodicall scheduled updater thread. All asynchronously.

I have considered both of these options to perform the update safely. 

    ArrayList<String> tempArray = ModelJob.getDistinctMonths(user, true);       
    synchronized (monthArray) {
        monthArray = tempArray;
    }

or 

    synchronized (monthArray) {
        monthArray = ModelJob.getDistinctMonths(user, true);
    }

The idea behind the first one being that the ModelJob.getDistinctMonths(user, true); call is time consuming and I dont want to hold the synchrinzation to block for longer then it has to, only for the quick reassignment of the old array with the updated array. However seems to be a obfuscated, and I only want to do it if completly nessecary. Can anyone give me any insight into how the jvm handles this synchrization and weather or not doing the former will give me any increase in performanace? Basically Im asking if the jvm would block for the whole static ModelJob call, or if it is able to get away with only blocking for the reassignment and be safe, and if so, if it is smart enought to do so.

Upvotes: 2

Views: 300

Answers (5)

jtahlborn
jtahlborn

Reputation: 53694

assuming that you don't need the synchronization around the getDistinctMonths() call (that call is thread-safe, and you don't need atomicity around the call and the assignment), then you can just synchronize around the assignment (and yes, the blocking is only scoped to the synchronized block, otherwise the syntax would be pointless). Note, @JohnVint brings up a good point that you should not synchronize on the monthArray reference as it is being modified. you must synchronize on a separate object instance which does not change.

Lastly, you could remove the synchronization block and make the monthArray member volatile and achieve the same results.

Upvotes: 3

Shilpa
Shilpa

Reputation: 106

Using the first approach will be better from performance point of view. It will avoid unnecessary synchronization.

One thing you have to remember is that even the read on monthArray will need to be synchronized. Synchronization only works when both update and read are synchronized using the same object lock. I would prefer to use the class Object lock. For example, if this code is a part of class ModelUpdate then use following code

synchronized(ModelUpdate.class) {
        monthArray = tempArray;
}

Upvotes: 1

John Vint
John Vint

Reputation: 40276

One obvious flaw here is that you should not synchronize on objects that will be changing.

I would prefer the first example but instead use a common lock

final Object lock = new Object();

synchronized(lock){
   monthArr = ...;
}

However as most said declaring monthArr volatile will have the same effects.

Upvotes: 1

Peter Ilfrich
Peter Ilfrich

Reputation: 3816

A synchronized block will always block for its entire execution. The object (monthArray in your case) given as the parameter is called the "monitor" and will guarantee, that all other synchronized blocks with the same object (monthArray) as parameter will be executed asynchronously.

Upvotes: 1

Marko Topolnik
Marko Topolnik

Reputation: 200296

If you just put the volatile modifier on the monthArray and remove all synchronized blocks, you'll have lock-free thread safety.

Also, the JVM may optimize your cleaner (second) version of code to execute as if it was the first version. So, if keeping locks, better stick to the cleaner version.

Upvotes: 1

Related Questions