Aggiunta a AtomicInteger in ConcurrentHashMap - java, multithreading, concurrency, java.util.concurrent, concurrenthashmap

Ho definito quanto segue

private ConcurrentMap<Integer, AtomicInteger>  = new ConcurrentHashMap<Integer, AtomicInteger>();

private void add() {
staffValues.replace(100, staffValues.get(100), new AtomicInteger(staffValues.get(100).addAndGet(200)));
}

Dopo il test, i valori che sto ottenendo non lo sonoatteso, e penso che ci sia una condizione di gara qui. Qualcuno sa se questo sarebbe considerato di sicurezza chiudendo la chiamata get nella funzione di sostituzione?

risposte:

2 per risposta № 1

Ci sono alcuni problemi con il tuo codice. Il più grande è che stai ignorando il valore di ritorno di ConcurrentHashMap.replace: se la sostituzione non avviene (a causa di un altro thread che ha effettuato una sostituzione in parallelo), si procede semplicemente come se è successo. Questa è la ragione principale per cui stai ottenendo risultati sbagliati.

Penso anche che sia un errore di progettazione mutare a AtomicInteger e poi immediatamente sostituirlo con un altro AtomicInteger; anche se riesci a farlo funzionare, semplicemente non c'è motivo per questo.

Infine, non penso che dovresti chiamare staffValues.get(100) due volte. Non credo che questo causi un bug nel codice corrente - la tua correttezza dipende solo dalla seconda chiamata che restituisce un risultato "più nuovo" rispetto al primo, che penso è effettivamente garantito da ConcurrentHashMap - ma è fragile, sottile e confuso In generale, quando chiami ConcurrentHashMap.replace, il suo terzo argomento dovrebbe essere qualcosa che hai calcolato usando il secondo.

Nel complesso, puoi semplificare il tuo codice non usando AtomicInteger:

private ConcurrentMap<Integer, Integer> staffValues = new ConcurrentHashMap<>();

private void add() {
final Integer prevValue = staffValues.get(100);
staffValues.replace(100, prevValue, prevValue + 200);
}

o non usando replace (e forse nemmeno ConcurrentMap, a seconda di come stai toccando questa mappa):

private Map<Integer, AtomicInteger> staffValues = new HashMap<>();

private void add() {
staffValues.get(100).addAndGet(200);
}

1 per risposta № 2

Un buon modo per gestire situazioni come questa sta usando il computeIfAbsent metodo (non il compute metodo consigliato da @ the8472)

Il computeIfAbsent accetta 2 argomenti, la chiave e a Function<K, V> verrà chiamato solo se il valore esistente è mancante. Poiché AtomicInteger è thread-safe per l'incremento da più thread, è possibile utilizzarlo facilmente nel modo seguente:

staffValues.computeIfAbsent(100, k -> new AtomicInteger(0)).addAndGet(200);

0 per risposta № 3

Non hai bisogno di usare replace(). AtomicInteger è un valore mutabile che non ha bisogno di essere sostituito ogni volta che si desidera incrementarlo. Infatti addAndGet già lo incrementa.

Invece usa compute inserire un valore predefinito (presumibilmente 0) nella mappa quando nessuno è presente e in caso contrario ottenere il valore preesistente e incrementarlo.

Se, d'altra parte, vuoi usare valori immutabili messi Integer istanze invece di AtomicInteger nella mappa e aggiornarli con le operazioni di calcolo / sostituzione / fusione atomiche.