Ajout à AtomicInteger dans ConcurrentHashMap - java, multithreading, concurrency, java.util.concurrent, concurrhashmap

J'ai la définition suivante

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

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

Après les tests, les valeurs que je reçois ne sont pasattendu, et je pense qu'il y a une condition de course ici. Est-ce que quelqu'un sait si cela serait considéré threadsafe en enveloppant l'appel get dans la fonction de remplacement?

Réponses:

2 pour la réponse № 1

Il y a quelques problèmes avec votre code. Le plus important est que vous ignorez la valeur de retour de ConcurrentHashMap.replace: si le remplacement ne se produit pas (en raison d'un autre thread ayant fait un remplacement en parallèle), il vous suffit de procéder comme si c'est arrivé. C'est la principale raison pour laquelle vous obtenez des résultats erronés.

Je pense aussi que c'est une erreur de conception de muter un AtomicInteger et ensuite le remplacer immédiatement par un autre AtomicInteger; Même si vous pouvez obtenir ce travail, il n'y a simplement aucune raison pour cela.

Enfin, je ne pense pas que vous devriez appeler staffValues.get(100) deux fois. Je ne pense pas que cela provoque un bug dans le code actuel - votre exactitude dépend uniquement du second appel qui retourne un résultat "plus récent" que le premier, ce que je pense est effectivement garanti par ConcurrentHashMap - mais il est fragile et subtil et déroutant. En général, quand vous appelez ConcurrentHashMap.replace, son troisième argument devrait être quelque chose que vous avez calculé en utilisant le second.

Dans l'ensemble, vous pouvez simplifier votre code en n'utilisant pas AtomicInteger:

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

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

ou en n'utilisant pas replace (et peut-être même pas ConcurrentMap, en fonction de la façon dont vous touchez cette carte):

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

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

1 pour la réponse № 2

Un bon moyen de gérer des situations comme celle-ci utilise le computeIfAbsent méthode (pas la compute méthode recommandée par @ the8472)

le computeIfAbsent accepte 2 arguments, la clé et un Function<K, V> cela ne sera appelé que si la valeur existante est manquante. Comme AtomicInteger peut être incrémenté à partir de plusieurs threads, vous pouvez l'utiliser facilement de la manière suivante:

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

0 pour la réponse № 3

Vous n'avez pas besoin d'utiliser replace(). AtomicInteger est une valeur mutable qui n'a pas besoin d'être substituée chaque fois que vous voulez l'incrémenter. En réalité addAndGet l'incrémente déjà en place.

Au lieu d'utiliser compute pour mettre une valeur par défaut (vraisemblablement 0) dans la carte quand aucun n'est présent et sinon obtenir la valeur préexistante et l'incrémenter.

Si, d'un autre côté, vous voulez utiliser des valeurs immuables Integer instances au lieu de AtomicInteger dans la carte et les mettre à jour avec les opérations atomiques de calcul / remplacement / fusion.