Требуется ли синхронизация ConcurrentHashMap при увеличении ее значений?

Question

Я знаю, что ConcurrentHashMap является поточно-безопасным, например, для параметра «Идентификатор», «Заменить» и т. Д., Но мне было интересно, это блок кода, подобный безопасному?

if (accumulator.containsKey(key)) { //accumulator is a ConcurrentHashMap 
    accumulator.put(key, accumulator.get(key)+1); 
} else { 
    accumulator.put(key, 0); 
} 

Имейте в виду, что значение аккумулятора для ключа может быть предложено два разных потоков одновременно, что может вызвать проблемы в нормальном HashMap. Так мне нужно что-то вроде этого?

ConcurrentHashMap<Integer,Object> locks; 
... 
locks.putIfAbsent(key,new Object()); 
synchronized(locks.get(key)) { 
    if (accumulator.containsKey(key)) { 
     accumulator.put(key, accumulator.get(key)+1); 
    } else { 
     accumulator.put(key, 0); 
    } 
} 

Answer 1

if (accumulator.containsKey(key)) { //accumulator is a ConcurrentHashMap 
    accumulator.put(key, accumulator.get(key)+1); 
} else { 
    accumulator.put(key, 0); 
} 

Нет, этот код не является потокобезопасным; accumulator.get(key) может быть изменен между get и put, или запись может быть добавлена ​​между containsKey и put. Если вы находитесь на Java 8, вы можете написать accumulator.compute(key, (k, v) -> (v == null) ? 0 : v + 1) или любой из многих эквивалентов, и он будет работать. Если это не так, что нужно сделать, это написать что-то вроде

while (true) { 
    Integer old = accumulator.get(key); 
    if (old == null) { 
    if (accumulator.putIfAbsent(key, 0) == null) { 
     // note: it's a little surprising that you want to put 0 in this case, 
     // are you sure you don't mean 1? 
     break; 
    } 
    } else if (accumulator.replace(key, old, old + 1)) { 
    break; 
    } 
} 

... что петли до тех пор, пока не удается сделать атомную замену. Этот вид цикла в значительной степени связан с тем, как у вас есть для этого: так работает AtomicInteger, а то, что вы просите, - AtomicInteger на многих клавишах.

В качестве альтернативы вы можете использовать библиотеку: например. В Guava есть AtomicLongMap и ConcurrentHashMultiset, которые также делают такие вещи.

Answer 2

Вы правы, что ваш первый фрагмент кода небезопасен. Совершенно разумно, чтобы поток был прерван сразу после проверки, а другой поток начал выполняться. Поэтому в первом фрагменте следующее может произойти:

[Thread 1]: Check for key, return false 
[Thread 2]: Check for key, return false 
[Thread 2]: Put value 0 in for key 
[Thread 1]: Put value 0 in for key 

В этом примере поведение вы хотели бы оставить вас в состоянии со значением для этого ключа быть установлен в 1, а не 0.

Поэтому необходимо блокирование.

Answer 3

Только отдельные действия на ConcurrentHashMap являются потокобезопасными; выполнение нескольких действий в последовательности - нет. Ваш первый блок кода не является потокобезопасным. Можно, например:

THREAD A: accumulator.containsKey(key) = false 
THREAD B: accumulator.containsKey(key) = false 
THREAD B: accumulator.put(key, 0) 
THREAD A: accumulator.put(key, 0) 

Similary, не поточно- получить значение аккумулятора для данного ключа, увеличить его, а затем положить его обратно на карте. Это трехэтапный процесс, и другой поток может прерываться в любой точке.

Ваш второй синхронизированный блок кода является потокобезопасным.

Answer 4

Я думаю, что лучшим решением для вас будет использование AtomicInteger. Приятная особенность здесь заключается в том, что она неблокирующая, изменяемая и потокобезопасная. Вы можете использовать метод replace, предлагаемый CHM, но с этим вам нужно будет удерживать блокировку сегмента/ведра до завершения замены.

С помощью AtomicInteger вы используете быстрые неблокирующие обновления.

ConcurrentMap<Key, AtomicInteger> map; 

затем

map.get(key).incrementAndGet(); 

---

Если вы используете Java 8, LongAdder будет лучше.