Multi-threading -- a faster way?

Question

I have a class with a getter getInt() and a setter setInt() on a certain field, say field

Integer Int; 

of an object of a class, say SomeClass.

The setInt() here is synchronized-- getInt() isn't.

I am updating the value of Int from within multiple threads. Each thread is getting the value Int, and setting it appropriately. The threads aren't sharing any other resources in any way.

The code executed in each thread is as follows.

public void update(SomeClass c) {

        while (<condition-1>) // the conditions here and the calculation of 
                                   // k below dont have anything to do 
                                   // with the members of c
            if (<condition-2>) {
                // calculate k here 
                synchronized (c) {                        
                        c.setInt(c.getInt()+k); 
                    //    System.out.println("in "+this.toString());
                }
            }  
}   

The run() method is just invoking the above method on the members updated from within the constructor by the params passed to it:

public void run() { update(c); } 

When I run this on large sequences, the threads aren't interleaving much-- i see one thread executing for long without any other thread running in between.

There must be a better way of doing this.

I can't change the internals of SomeClass, or of the class invoking the threads.

How can this be done better?

TIA.

//=====================================

EDIT:

I'm not after manipulating the execution sequence of the threads. They all have the same priority. It`s just that what i see in the outcome is suggesting that the threads aren't sharing the execution time evenly-- one of them, once takes over, executing on. However, I can't see why this code should be doing this.

Answer 1

As Enno Shioji has pointed out, letting one thread proceed might be the most efficient way to execute your code in some scenarios.

It depends on how much cost the thread synchronization imposes in relation to the other work of your code (which we don’t know). If you have a loop like:

while (<condition-1>)
    if (<condition-2>) {
        // calculate k here 
        synchronized (c) {                        
            c.setInt(c.getInt()+k); 
        }
    }

and the test for condition-1 and condition-2 and the calculation of k is rather cheap compared to the synchronization cost, the Hotspot optimizer might decide to reduce the overhead by transforming the code to something like this:

synchronized (c) {                        
    while (<condition-1>)
        if (<condition-2>) {
            // calculate k here 
            c.setInt(c.getInt()+k); 
        }
}

(or a rather more complicated structure by performing loop unrolling and span the synchronized block over multiple iterations). The bottom line is that the optimized code might block other threads longer but let the one owning the lock finish faster resulting in an overall faster execution.

This does not mean that a single-threaded execution was the fastest way to handle your problem. It also doesn’t mean that using an AtomicInteger here would be the best option to solve the problem. It would create a higher CPU load and possibly a small acceleration but it doesn’t solve your real mistake:

It is completely unnecessary to update c within the loop at a high frequency. After all, your threads do not depend on seeing updates to c timely. It even looks like they are not using it at all. So the correct fix would be to move the update out of the loop:

int kTotal=0;
while (<condition-1>)
    if (<condition-2>) {
        // calculate k here
        kTotal += k;
    }
synchronized (c) {                        
    c.setInt(c.getInt()+kTotal); 
}

Now, all threads can run in parallel (assuming the code you haven’t posted here doesn’t contain inter-thread dependencies) and the synchronization cost is reduced to a minimum. You could still change it to an AtomicInteger as well but that’s not that important anymore.

Answer 2

I've found it better to use wait() and notify() than yield. Check out this example (seen from a book)-

class Q {
int n;
boolean valueSet = false;
synchronized int get() {
if(!valueSet)
wait(); //handle InterruptedException
//
valueSet = false;
notify();//if thread waiting in put, now notified
}

synchronized void put(int n) {
if(valueSet)
wait(); //handle InterruptedException
//
valueSet = true;
//if thread in get waiting then that is resumed now
notify();
}
}

or you could try using sleep() and join the threads in the end in main() but that isn't a foolproof way

Answer 3

It`s just that what i see in the outcome is suggesting that the threads aren't sharing the execution time evenly

Well, this is exactly what you don't want if you are after efficiency. Yanking a thread from being executed and scheduling another thread is generally very costly. Therefore it's actually advantageous to do one of them, once takes over, executing on. Of course, when this is overdone you could see higher throughput but longer response time. In theory. In practice, JVMs thread scheduling is well tuned for almost all purposes, and you don't want to try changing it in almost all situations. As a rule of thumb, if you are interested in response times in millisecond order, you probably want to stay away messing with it.

tl;dr: It's not being inefficient, you probably want to leave it as it is.

EDIT:
Having said that, using an AtomicInteger may help in performance, and is in my opinion less error prone than using a lock (synchronized keyword). You need to be hitting that variable really very hard in order to get a measurable benefit though.

Answer 4

You are having public void update(SomeClass c) method in your code and this method is an instance method in which you are passing the object as parameter.

synchronized(c) in your code is doing nothing. Let me show you with some example,

So if you will make different objects of this class and then try to make them different threads like,

   class A extends Thread{
          public void update(SomeClass c){}
          public void run(){
               update(c)
          }
          public static void main(String args[]){

               A t1 = new A();
               A t2 = new A();

               t1.start();
               t2.start();

          }
   }

Then both of these t1 & t2 will have their own copies of update method and the reference variable c which you are making synchronized will also be different for both the threads. t1 calls its own update() method and t2 calls its own update() method. So synchronization won't work.

Synchronization will work when you have something common for both the threads.

Something like,

   class A extends Thread{

          static SomeClass c;

          public void update(){
                synchronized(c){

                }
          }
          public void run(){
               update(c)
          }
          public static void main(String args[]){

               A t1 = new A();
               A t2 = new A();

               t1.start();
               t2.start();

          }
   }

This way the actual concept of synchronization will be applied.

Answer 5

The JDK provides a nice solution for multi threaded int access, AtomicInteger:

http://docs.oracle.com/javase/7/docs/api/java/util/concurrent/atomic/AtomicInteger.html

Answer 6

Answering to this

i see one thread executing for long without any other thread running in between. There must be a better way of doing this.

You can not control how threads will be executed. JVM does this for you, and does not like you to interfere in its work.

Still you can look at yield as your option, but that also does not ensure same thread will not be picked again.

The java.lang.Thread.yield() method causes the currently executing thread object to temporarily pause and allow other threads to execute.