0

Firstly I'm not using iterator here.

I'm using 2 threads on an shared ArrayList ,1st is used to adding values into ArrayList and other is to make a temporary copy of it and do some operations on it and then removing all temp elements from original list.

import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;

public class ArraylistTest {

    public static void main(String...ar){
        new AddingThread().start();
        new RemovalThread().start();
    }
}

class RemovalThread extends Thread{
    static List<Integer> originalBigList = new ArrayList<>();

    @Override
    public void run(){
        System.out.println("RemovalThread started");
        while(true){
            try {
                sleep(1000);
            } catch (InterruptedException e) {
                e.printStackTrace();
            }
            System.out.println("creating copy of originalBigList");
            List<Integer> tempList = new ArrayList<>(originalBigList);
            System.out.println("copied list");
            //
            //some operations on copied temp list
            //
            System.out.println("removing tempList elements after completing operations");
            System.out.println("originalBigList before removing size "+originalBigList.size());
            originalBigList.removeAll(tempList);
            System.out.println("removed!!");
            System.out.println("after size "+originalBigList.size());
        }
    }
}

class AddingThread extends Thread{

    @Override
    public void run(){
        System.out.println("Adding thread started");
        int ctr = 0;
        while(true){
            RemovalThread.originalBigList.add(ctr);
            ctr++;
        }
    }
}

Output :-

Adding thread started
RemovalThread started
creating copy of originalBigList
copied list
removing tempList elements after completing operations
originalBigList before removing size 4102267
Exception in thread "Thread-0" java.lang.OutOfMemoryError: Java heap space
    at java.util.Arrays.copyOf(Arrays.java:3210)
    at java.util.Arrays.copyOf(Arrays.java:3181)
    at java.util.ArrayList.grow(ArrayList.java:261)
    at java.util.ArrayList.ensureExplicitCapacity(ArrayList.java:235)
    at java.util.ArrayList.ensureCapacityInternal(ArrayList.java:227)
    at java.util.ArrayList.add(ArrayList.java:458)
    at AddingThread.run(ArraylistTest.java:47)

Now my question is that i'm seeing in the output that the statement which is copying the list is executed which is making temp list from original list, but removal statement is not executing and not giving any Exception and i'm using simple arraylist not synchronised,so why is it so?

Is there any internal lock on remove or add operation? if yes then what is the use of Collections.synchronised( arraylist )?

Rohan Dodeja
  • 296
  • 1
  • 5
  • 18

3 Answers3

2

The following line

            List<Integer> tempList = new ArrayList<>(originalBigList);

Will iterate inside list constructor over originalBigList to create your tempList and if AddingThread will run in paralel it will throw the ConcurrentModificationException that kills your RemovalThread maybe you don't see it in console but it is probably there. I would sugest to use CopyOnWriteArrayList for your originalBigList

urag
  • 1,228
  • 9
  • 28
  • i also thought that it will give ConcurrentModification Exception as according to Java doc but it doesn't give I run this code several times and i did always gives same output. – Rohan Dodeja Jun 27 '17 at 12:56
  • How do you know it did not did you tried to put try catch over all of the of the block inside while and print the exception? Because if you do it while debuging it may act differently – urag Jun 27 '17 at 12:59
  • OK i haven't put try catch but if it occurs then it should be catched by runtime environment of JVM and will be shown at console . but it does't, and also if i use CopyOnWriteArrayList then i have heavy write ops so on every write it will create a new list , i think it is not feasible. – Rohan Dodeja Jun 27 '17 at 13:06
  • "Note that the fail-fast behavior of an iterator cannot be guaranteed as it is, generally speaking, impossible to make any hard guarantees in the presence of unsynchronized concurrent modification. Fail-fast iterators throw ConcurrentModificationException on a best-effort basis. Therefore, it would be wrong to write a program that depended on this exception for its correctness: the fail-fast behavior of iterators should be used only to detect bugs." according to https://docs.oracle.com/javase/8/docs/api/java/util/ArrayList.html – Emre Sevinç Jun 27 '17 at 13:06
  • I mentioned that i'm not using Iterator , please go through the code and try to run it, i'm not getting ConcurentModification Exception ever. The output always shows that it can't able to execute remove command and the adding thread is adding to originalList – Rohan Dodeja Jun 27 '17 at 13:17
2

I found your problem look at the following code

 public static void main(String ...args){
    long start = System.currentTimeMillis();
    List<Integer> l1 = new ArrayList<>();
    List<Integer> l2 = new ArrayList<>();

    for(int i = 0 ; i < Integer.MAX_VALUE/10000;i++){
        l1.add(i);
        l2.add(i);
    }

    System.out.println(String.format("Field both arrays in %s seconds",(System.currentTimeMillis()-start)/1000) );
    start = System.currentTimeMillis();
    l1.removeAll(l2);

    System.out.println(String.format("Removed one array from other %s seconds",(System.currentTimeMillis()-start)/1000) );

}

Field both arrays in 0 seconds

Removed one array from other 34 seconds

It is a huge array now look at the output remove toke a huge amount of time and in your case, AddingThread makes the array huge so it's working it just takes a lot of time and meanwhile, AddingThread keeps working.By the way, it is understandable why it takes so long each value needs to be found first so it's O(n^2).Look how it works when I use HashSet instead of List (note:HashSet search is O(1))

public class HashSetTest {
public static Object obj = new Object();
public static void main(String... ar) {
    new AddingThread().start();
    new RemovalThread().start();
}

}

class RemovalThread extends Thread {
static Set<Integer> originalBigSet = new HashSet<>();
@Override
public void run() {
    System.out.println("RemovalThread started");
    while (true) {
        try {
            sleep(1000);
            System.out.println("creating copy of originalBigSet");
            Set<Integer> tempList;
            synchronized (HashSetTest.obj) {
                tempList = new HashSet<>(originalBigSet);
            }
            System.out.println("copied list");
            //
            //some operations on copied temp list
            //
            System.out.println("removing tempList elements after completing operations");
            System.out.println("originalBigSet before removing size " + originalBigSet.size());
            synchronized (HashSetTest.obj) {
                originalBigSet.removeAll(tempList);
            }
            System.out.println("removed!!");
            System.out.println("after size " + originalBigSet.size());
        } catch (Exception e) {
            e.printStackTrace();
        }
    }
}

}

class AddingThread extends Thread {

@Override
public void run() {
    System.out.println("Adding thread started");
    int ctr = 0;
    while (true) {
        synchronized (HashSetTest.obj) {
            RemovalThread.originalBigSet.add(ctr);
        }
        ctr++;
    }
}

}

urag
  • 1,228
  • 9
  • 28
1

Similar to the answer of @urag (especially the remark about being O(n²)), you can see the effect by modifying AddingThread like, so it adds only a limited number of items:

class AddingThread extends Thread{

    @Override
    public void run(){
        System.out.println("Adding thread started");
        int ctr = 0;
        while(true){

          if (ctr < 100_000) {
            RemovalThread.originalBigList.add(ctr);
            ctr++;
           }
      }
  }
}

This results in:

Adding thread started
RemovalThread started
creating copy of originalBigList
copied list
removing tempList elements after completing operations
originalBigList before removing size 100000
removed!!
after size 0

Now, change 100_000 to 1_000_000, and you'll wait for a long time, and only see::

Adding thread started
RemovalThread started
creating copy of originalBigList
copied list
removing tempList elements after completing operations
originalBigList before removing size 1000000
Emre Sevinç
  • 8,211
  • 14
  • 64
  • 105