Java concurrency, thread behavior
Methods getFirst()
and getSecond()
this class are called simultaneously. This is part of the web application.
Internal cards are also filled with no concurrency.
public class MyClass {
private Map<String, List<List<String>>> first;
private Map<String, List<List<String>>> second;
public MyClass() {
first = new ConcurrentHashMap<>();
second = new ConcurrentHashMap<>();
}
public Set<String> getFirst(String key, String token, int a, int b) {
return get(first, key, token, a, b);
}
public Set<String> getSecond(String key, String token, int a, int b) {
return get(second, key, token, a, b);
}
private Set<String> get(final Map<String, List<List<String>>> map, final String key, final String token,
final int a, final int b) {
Set<String> result = new TreeSet<>();
map.get(key).stream().filter(i -> i.size() <= b && i.size() >= a).forEach(
s -> result
.addAll(s.stream().filter(p -> StringUtils.containsIgnoreCase(p, token)).collect(Collectors.toList())));
return result;
}
}
I've tested it with something like ab -n 10000 -c 100
(Apache utility). And I will write it down. I get the same set all the time. But if I change map.get(key).stream()
to map.get(key).parallelStream()
and follow the same steps, sometimes I get a different result size (always smaller).
What is it?
source to share
You are using TreeSet.addAll()
inside forEach
parallel thread. The body forEach
can execute concurrently multiple times in different threads for different elements, and is TreeSet
not thread safe. To quickly fix the problem, you can either sync the changes result
or use forEachOrdered
. However, it would be cleaner and more efficient to use flatMap
your stream and collect it right away without forEach
. Try this version:
return map.get(key).stream()
.filter(i -> i.size() <= b && i.size() >= a)
.flatMap(List::stream).filter(p -> StringUtils.containsIgnoreCase(p, token))
.collect(Collectors.toCollection(TreeSet::new));
source to share