I am splitting the String List in main into 2 different Threads to map the words inside.
Every time I execute this code I get different mapping results.
Either I have a BIG logic flaw, or there is something I am missing about Threads and Concurrent Collections.
Can anyone understand why is this happening?
There are 8 "a" 's and 6 "b" 's added to the list.
P.S. this does not happen If I use ONE Thread only!
EDIT 1
changing map.put() to map.merge(word, 1, Integer::sum), Still doesnt work
EDIT 2
following solution I didnt use if/else, only merge and it works as expected.
public class MyThread extends Thread {
private List<String> list;
private final ConcurrentHashMap<String, Integer> map;
public MyThread(ConcurrentHashMap<String, Integer> map, List<String> list) {
this.map = map;
this.list = list;
}
@Override
public void run() {
for (String word : list){
map.merge(word, 1, Integer::sum);
}
}
public ConcurrentHashMap<String, Integer> getMap() {
return map;
}
}
public static void main(String[] args) throws InterruptedException {
ConcurrentHashMap<String, Integer> map = new ConcurrentHashMap<>();
List<String> list = new ArrayList<>();
list.add("a");list.add("a");list.add("a");list.add("a");list.add("b");list.add("b");list.add("b");
list.add("a");list.add("a");list.add("a");list.add("a");list.add("b");list.add("b");list.add("b");
MyThread[] ts = new MyThread[2];
int start = 0;
int end = list.size()/2;
for (int i = 0; i < 2; i ){
ts[i] = new MyThread(map,new ArrayList<>(list.subList(start, end)));
ts[i].start();
start = end;
end = list.size();
}
for (int i = 0; i < 2; i ){
ts[i].join();
}
for(String word : map.keySet()){
System.out.println("Key = " word ". Value = " map.get(word));
}
}
CodePudding user response:
First thing - you should use map.containsKey(word)
instead map.contains(word)
.
About different results after some runnings. There is no atomically code when you invoke containsKey()
and merge()
methods. Both of them atomically separately, but are not atomically together.
The solution is just to change your code and call only merge()
which is atomic. You don't need if-else
section at all.
@Override
public void run() {
for (String word : list) {
map.merge(word, 1, Integer::sum);
}
}