Creating 1000 Threads to change a variable value

Question

I have created a program which creates 1000 threads and each threads adds 1 in to a variable sum. My problem is the output i get is only 1s.

Here is the program:

class Threading implements Runnable{

    T6_Q1 sumObject=new T6_Q1();

    Thread t;

    Threading(){
        t=new Thread(this);
        t.start();
    }

    @Override
    public void run() {

        setSumValue();

        System.out.println(sumObject.getSum());   
    }


   public void setSumValue(){
        Integer value=sumObject.getSum().intValue()+1;

        sumObject.setSum(value);

    }
}

public class T6_Q1
{


Integer sum =new Integer(0);

public void setSum(int value){
   this.sum=new Integer(value);
}

//method to get the sum value
public Integer getSum(){
    return this.sum;
}

public static void main(String[] args) {

   //launches 1000 threads
    for(int i=1;i<=1000;i++)
    {
    new Threading();
    }

}
}

Even if i synchronized the setSumValue method i only get 1s. What am i doing wrong here ? (Am new to threading so still bit hard to understand the errors)

Thank you for your time.

score 7 · Accepted Answer · answered Sep 01 '13 at 13:20

7

Each of your Threading objects has its own sumObject instance, with its own value.
They have nothing to do with eachother.

Once you fix that, you will discover that your code is completely non-thread-safe.
Synchronizing on 1,000 different objects will not fix that at all; synchronizing on a single object will completely defeat the purpose of the threads.

answered Sep 01 '13 at 13:20

SLaks

868,454
176
1,908
1,964

I'd agree with everything except the "defeat the purpose" part. This is obviously just a minimal example, in the wider context a thread-safe version would make good sense. – Marko Topolnik Sep 01 '13 at 13:41
@MarkoTopolnik: No; `AtomicInteger`. – SLaks Sep 01 '13 at 13:42
Alright, that's a Java pro tip, and works only when this example is taken literally. Maybe there's more work than just mutating a single variable. Also consider that locks are actually more efficient than CAS under high load. – Marko Topolnik Sep 01 '13 at 13:49
You are correct i cannot synchronize all the 1000 objects just by adding synchronize to the method. How can i synchronize the object to get the values printed from 1 - 1000? I tried using synchronized(sumObject) inside the setSumValue method as well but it doesn't work. Thanks again :) – Troller Sep 01 '13 at 14:07
@Marko How can a lock be more efficient than a CAS under high load? Basically: How the hell do you implement a lock without some kind of atomic instruction to begin with? – Voo Sep 01 '13 at 15:15
@Voo The key lies in the essential difference between optimistic and pessimistic transaction isolation. Incementing a counter by CAS can be implemented only with a potentially infinite loop of update retries. So with the CAS approach you bet on low incidence of retries. Your odds progressively deteriorate as contention grows, until the point where it pays more to just say "everybody get in the line and wait your turn". – Marko Topolnik Sep 01 '13 at 16:02
@Marko Ah so what you mean is that it's cheaper to once lock and then do several instructions normally than to have several atomic instructions. That one I agree with. But locking will never be more efficient than a single CAS instruction as far as I can see. – Voo Sep 01 '13 at 16:30
@Voo That's true, I meant more than just one CAS (which is actually a single CPU instruction); I meant the unit of work involving CAS, which is almost always coupled with a retry loop. – Marko Topolnik Sep 01 '13 at 16:34
@Marko Still if you want to update only a single value, a CAS loop will be more efficient than locking and then updating it, since as far as I can see there is no lock algorithm that doesn't need some kind of atomic instruction themselves (how would that work?) – Voo Sep 01 '13 at 17:17
@Voo My guess is, it is only a single CAS, no loop because its key feature is to be a *blocking* operation. A single attempt is made; if it fails, the thread registers itself under "BLOCKED" state. – Marko Topolnik Sep 01 '13 at 18:45
@Marko Makes sense. Single CAS and then context switch to Kernel. Still not sure if it's quicker under the assumption that one thread succeeds each CAS round (no idea if that's necessarily true or not). I'll have to try that sometime on one of the larger machines with a few hundred cores. – Voo Sep 02 '13 at 11:35

Creating 1000 Threads to change a variable value

1 Answers1