Безопасное обновление ConcurrentHashMap и AtomicInteger - PullRequest
Новая
       290

Безопасное обновление ConcurrentHashMap и AtomicInteger

2 голосов
/ 21 ноября 2011

Я должен хранить слова и соответствующие им целочисленные индексы в хэш-карте.Хеш-карта будет обновляться одновременно.

Например: допустим, что wordList равно {a,b,c,a,d,e,a,d,e,b} Карта хеша будет содержать следующие пары ключ-значение 

a:1
b:2
c:3
d:4
e:5

Код для этого следующий:* 

public class Dictionary {

private ConcurrentMap<String, Integer>  wordToIndex;
private AtomicInteger                   maxIndex;

public Dictionary( int startFrom ) {
    wordToIndex = new ConcurrentHashMap<String, Integer>();
    this.maxIndex = new AtomicInteger(startFrom);
}


public void insertAndComputeIndices( List<String> words ) {

    Integer index;
    //iterate over the list of words
    for ( String word : words ) {
        // check if the word exists in the Map
        // if it does not exist, increment the maxIndex and put it in the
        // Map if it is still absent
        // set the maxIndex to the newly inserted index

        if (!wordToIndex.containsKey(word)) {
            index = maxIndex.incrementAndGet();

            index = wordToIndex.putIfAbsent(word, index);
            if (index != null)
                maxIndex.set(index);
        }
    }
}

Мой вопрос: является ли указанный класс потокобезопасным или нет?По сути, в этом случае атомарная операция должна увеличивать maxIndex, а затем помещать слово в хэш-карту, если она отсутствует.

Есть ли лучший способ достижения параллелизма в этой ситуации?

Ответы [ 4 ]

3 голосов
/ 21 ноября 2011

Ясно, что другой поток может видеть, что maxIndex увеличивается, а затем становится засоренным.

Если предположить, что это все, что происходит на карте (в частности, без удалений), тогда вы можете попробовать поместить слово на карту и увеличивать его только в случае успеха. 

    Integer oldIndex = wordToIndex.putIfAbsent(word, -1);
    if (oldIndex == null) {
        wordToIndex.put(word, maxIndex.incrementAndGet());
    }

(В качестве альтернативы для одного put используйте какой-нибудь изменчивый тип вместо Integer.)

3 голосов
/ 21 ноября 2011

Нет, это не так.Если у вас есть два метода A и B, оба безопасны для потоков, это, конечно, не означает, что вызовы A и B в последовательности также являются потокобезопасными, поскольку поток может прерывать другой между вызовами функции.Вот что происходит здесь: 

    if (!wordToIndex.containsKey(word)) {
        index = maxIndex.incrementAndGet();

        index = wordToIndex.putIfAbsent(word, index);
        if (index != null)
            maxIndex.set(index);
    }

Поток A проверяет, что wordToIndex не содержит слова «собака», и переходит в if.Прежде чем он сможет добавить слово «собака», поток B также обнаружит, что «собака» отсутствует на карте (А еще не добавила его), поэтому он также продолжается внутри оператора if.Теперь у вас есть слово «собака», которое вы пытаетесь вставить дважды.

Конечно, putIfAbsent гарантирует, что только один поток может добавить его, но я думаю, что ваша цель - не вводить два потока в if одновременно с одним и тем же ключом.

0 голосов
/ 21 ноября 2011

Остальные ответы правильные --- в вашем классе есть не поточнобезопасные поля.Для начала вам нужно убедиться, что

как реализовать создание потоков

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

2) Найдите любой из ваших методов доступа, убедитесь, что они синхронизированы всякий раз, когда изменяется состояние глобального объекта (ИЛИ ИМХО, ЕСЛИ БЛОК СИНХРОНИЗИРОВАН).

3) Проверка на наличие взаимоблокировок или неправильных подсчетов. Это можно реализовать в модульном тесте, убедившись, что значение maxIndex является правильным после 10000 резьбовых вставок, например ...

0 голосов
/ 21 ноября 2011

AtomicInteger - это то, что вы должны использовать.

И вы должны заключить весь код, который должен произойти, в transaction в блоке synchronized(this).

...