[FIXED] Why is count1 < count2 after running the following code? Both count1 and count2 should be 2000000

Issue

Compile and run in JDK11 and look at the results. sum1 is the time taken by the synchronized code and sum2 is the time for the AtomicInteger code. count1 is the result of counting the number of calls to the synchronized count++. count2 is the same number of combined calls but using AtomicInteger. The counts should be 2000000 and it is expected that sum1 > sum2 in ms. However, count1 is significantly less so where do the calls go?

github

package com.charlie;

import java.util.Date;
import java.util.concurrent.atomic.AtomicInteger;

public class AtomicTest extends Thread{

    private volatile Integer count1 = 0;
    private AtomicInteger count2 = new AtomicInteger(0);
    
    private void method1() {
        for (int i=0; i<1000; i++) {
            int a = i;
        }
        synchronized (count1){
            count1++;
        }
    }    
    private void method2() {
        for (int i=0; i<1000; i++) {
            int a = i;
        }
        count2.getAndIncrement();
    }
    
    public class Thread1 extends Thread{
        public Date start = null;
        public Date stop = null;
        @Override
        public
        void run() {
            start = new Date();
            for(int i=0;i<1000000;i++) {
                method1();
            }
            stop = new Date();
        } 
    }    
    public class Thread3 extends Thread{
        public Date start = null;
        public Date stop = null;
        @Override
        public
        void run() {
            start = new Date();
            for(int i=0;i<1000000;i++) {
                method2();
            }
            stop = new Date();
        } 
    }    
    static Thread1 t1 = null;
    static Thread1 t2 = null;
    static Thread3 t3 = null;
    static Thread3 t4 = null;
    static AtomicTest master = null;

    public static void main(String[] args) {
        AtomicTest master = new AtomicTest();
        t1 = master.new Thread1();
        t2 = master.new Thread1();
        t3 = master.new Thread3();
        t4 = master.new Thread3();
        master.start();
        t1.start();t2.start();t3.start();t4.start();
    }
    
    @Override
    public void run() {
        //stop and sum
        Boolean finished = false;
        while(!finished) {
            try {
                AtomicTest.sleep(1000);
            } catch (InterruptedException e) {
                // TODO Auto-generated catch block
                e.printStackTrace();
            }
            if(t1.stop != null && t2.stop != null && t3.stop != null && t4.stop != null) {
                Long sum1 = (t1.stop.getTime() - t1.start.getTime()) 
                + (t2.stop.getTime() - t2.start.getTime());
                Long sum2 = (t3.stop.getTime() - t3.start.getTime())
                + (t4.stop.getTime() - t4.start.getTime());
                finished = true;
                System.out.println("sum1 =" + sum1 + "ms sum2 =" + sum2 + "ms");
                System.out.println("count1 =" + count1 + " count2 =" + count2);
            }
        }
    }
}

Solution

a Java Integer is immutable, so when you use ++ on it, it creates a new object. If you synchronize on these objects they will have different identities, hence this won't work. To fix it you can change the Integer to a plain int (if you want not necessary) but create a separate Object to synchronize on. e.g. private Object sync = new Object(); Changing from Integer to plain int should also speed things up.

This works changes are in github now. Thankyou Magyar Tamas as well as Jason Splashett.

Answered By - Charles Swires
Answer Checked By - Mildred Charles (JavaFixing Admin)