Синхронизированный блок Java против Collections.synchronizedMap

Установлен ли следующий код для правильной синхронизации вызовов на synchronizedMap?

public class MyClass {
  private static Map<String, List<String>> synchronizedMap = Collections.synchronizedMap(new HashMap<String, List<String>>());

  public void doWork(String key) {
    List<String> values = null;
    while ((values = synchronizedMap.remove(key)) != null) {
      //do something with values
    }
  }

  public static void addToMap(String key, String value) {
    synchronized (synchronizedMap) {
      if (synchronizedMap.containsKey(key)) {
        synchronizedMap.get(key).add(value);
      }
      else {
        List<String> valuesList = new ArrayList<String>();
        valuesList.add(value);
        synchronizedMap.put(key, valuesList);
      }
    }
  }
}

По моему мнению, мне нужен синхронизированный блок в addToMap(), чтобы предотвратить вызов другого потока из remove() или containsKey(), прежде чем я перейду к вызову put(), но мне не нужен синхронизированный блок в doWork() потому что другой поток не может войти в синхронизированный блок в addToMap(), прежде чем remove() вернется, потому что я создал карту первоначально с помощью Collections.synchronizedMap(). Это верно? Есть ли лучший способ сделать это?

Ответ 1

Collections.synchronizedMap() гарантирует, что каждая операция атома, которую вы хотите запустить на карте, будет синхронизирована.

Выполнение двух (или более) операций на карте, однако, должно быть синхронизировано в блоке. Итак, да - вы правильно синхронизируете.

Ответ 2

Если вы используете JDK 6, вы можете проверить ConcurrentHashMap

Обратите внимание на метод putIfAbsent в этом классе.

Ответ 3

Существует потенциал для тонкой ошибки в вашем коде.

[ ОБНОВЛЕНИЕ:. Поскольку он использует map.remove(), это описание не является полным. Я пропустил этот факт в первый раз.:( Спасибо автору вопроса за указание на это. Я оставляю остальное как есть, но изменил инструкцию, чтобы сказать, что есть потенциальная ошибка.]

В doWork() вы получаете значение List с карты в потокобезопасном режиме. Впоследствии, однако, вы получаете доступ к этому списку в небезопасном вопросе. Например, один поток может использовать список в doWork(), а другой поток вызывает synchronizedMap.get(key).add(значение) в addToMap(). Эти два доступа не синхронизированы. Эмпирическое правило состоит в том, что гарантии потоковой передачи коллекции не распространяются на ключи или значения, которые они хранят.

Вы можете исправить это, вставив в карту синхронизированный список, например

List<String> valuesList = new ArrayList<String>();
valuesList.add(value);
synchronizedMap.put(key, Collections.synchronizedList(valuesList)); // sync'd list

В качестве альтернативы вы можете синхронизироваться на карте во время доступа к списку в doWork():

  public void doWork(String key) {
    List<String> values = null;
    while ((values = synchronizedMap.remove(key)) != null) {
      synchronized (synchronizedMap) {
          //do something with values
      }
    }
  }

Последняя опция немного ограничит concurrency, но несколько более ясная IMO.

Кроме того, обратите внимание на ConcurrentHashMap. Это действительно полезный класс, но не всегда является подходящей заменой синхронизированным HashMaps. Цитата из его Javadocs,

Этот класс полностью совместим с Hashtable в программах, которые полагаются на безопасность потока, но не на его детали синхронизации.

Другими словами, putIfAbsent() отлично подходит для атомных вставок, но не гарантирует, что другие части карты не будут меняться во время этого вызова; он гарантирует только атомарность. В вашей примерной программе вы полагаетесь на детали синхронизации (синхронизированной) HashMap для других вещей, кроме put() s.

Последняя вещь.:) Эта отличная цитата из Java concurrency in Practice всегда помогает мне в разработке отладочных многопоточных программ.

Для каждой изменяемой переменной состояния, к которой может обращаться более одного потока, все обращения к этой переменной должны выполняться с той же блокировкой.

Ответ 4

Да, вы правильно синхронизируете. Я объясню это более подробно. Вы должны синхронизировать два или более вызова метода на объекте synchronizedMap только в том случае, когда вы должны полагаться на результаты предыдущих вызовов (-ов) метода в последующем вызове метода в последовательности вызовов методов объекта synchronizedMap. Давайте посмотрим на этот код:

synchronized (synchronizedMap) {
    if (synchronizedMap.containsKey(key)) {
        synchronizedMap.get(key).add(value);
    }
    else {
        List<String> valuesList = new ArrayList<String>();
        valuesList.add(value);
        synchronizedMap.put(key, valuesList);
    }
}

В этом коде

synchronizedMap.get(key).add(value);

synchronizedMap.put(key, valuesList);

вызовы методов зависят от результата предыдущего

synchronizedMap.containsKey(key)

вызов метода.

Если последовательность вызовов метода не была синхронизирована, результат может быть неправильным. Например, thread 1 выполняет метод addToMap() и thread 2 выполняет метод doWork() Последовательность вызовов методов объекта synchronizedMap может быть следующей: thread 1 выполнил метод

synchronizedMap.containsKey(key)

и результат "true". После этого операционная система переключила управление выполнением на thread 2 и выполнила

synchronizedMap.remove(key)

После этого управление выполнением переключилось на thread 1 и выполнило, например,

synchronizedMap.get(key).add(value);

полагая, что объект synchronizedMap содержит key и NullPointerException, потому что synchronizedMap.get(key) вернет null. Если последовательность вызовов метода на объекте synchronizedMap не зависит от результатов друг друга, вам не нужно синхронизировать последовательность. Например, вам не нужно синхронизировать эту последовательность:

synchronizedMap.put(key1, valuesList1);
synchronizedMap.put(key2, valuesList2);

Здесь

synchronizedMap.put(key2, valuesList2);

вызов метода не зависит от результатов предыдущего

synchronizedMap.put(key1, valuesList1);

(не имеет значения, вмешался ли какой-то поток между двумя вызовами метода и, например, удалил key1).

Ответ 5

Это выглядит правильно для меня. Если бы я что-то изменил, я бы прекратил использовать Collections.synchronizedMap() и синхронизировал все одинаково, просто чтобы сделать его более ясным.

Кроме того, я бы заменил

  if (synchronizedMap.containsKey(key)) {
    synchronizedMap.get(key).add(value);
  }
  else {
    List<String> valuesList = new ArrayList<String>();
    valuesList.add(value);
    synchronizedMap.put(key, valuesList);
  }

List<String> valuesList = synchronziedMap.get(key);
if (valuesList == null)
{
  valuesList = new ArrayList<String>();
  synchronziedMap.put(key, valuesList);
}
valuesList.add(value);

Ответ 6

Отъезд Коллекции Google 'Multimap, например. стр. 28 из эта презентация.

Если вы не можете использовать эту библиотеку по какой-либо причине, используйте ConcurrentHashMap вместо SynchronizedHashMap; он имеет отличный putIfAbsent(K,V) метод, с помощью которого вы можете атомизировать список элементов, если он еще не существует. Кроме того, рассмотрите возможность использования CopyOnWriteArrayList для значений карты, если ваши шаблоны использования гарантируют это.

Ответ 7

То, как вы синхронизированы, является правильным. Но есть улов

Синхронизированная обертка, предоставляемая Framework Collection, гарантирует, что метод вызывает I.e add/get/contains будет работать взаимоисключающим.

Однако в реальном мире вы обычно запрашиваете карту, прежде чем вводить значение. Следовательно, вам нужно будет выполнить две операции, и, следовательно, необходим синхронизированный блок. Таким образом, вы использовали его правильно. Однако.

Вы могли бы использовать параллельную реализацию Map, доступную в рамках Collection. Преимущество ConcurrentHashMap -

а. Он имеет API 'putIfAbsent', который будет делать то же самое, но более эффективным образом.

б. Эффективность: dCocurrentMap просто блокирует ключи, поэтому он не блокирует весь мир карты. Если вы заблокировали ключи, а также значения.

с. Вы могли бы передать ссылку на свой объект карты где-нибудь еще в своей кодовой базе, где вы/другой разработчик в своем тиане могут в конечном итоге использовать его неправильно. Я могу просто добавить add() или get() без блокировки на объект карты. Следовательно, его вызов не будет выполняться взаимно исключающим образом для вашего блока синхронизации. Но использование параллельной реализации дает вам спокойствие, что это никогда не могут быть использованы/реализованы неправильно.