Java синхронизированный блок против Collections.synchronizedMap - PullRequest
81 голосов
/ 19 февраля 2009

Установлен ли следующий код для правильной синхронизации вызовов на 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(). Это верно? Есть ли лучший способ сделать это?

Ответы [ 7 ]

87 голосов
/ 19 февраля 2009

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

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

14 голосов
/ 19 февраля 2009

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

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

13 голосов
/ 20 февраля 2009

В вашем коде есть потенциал для незначительной ошибки.

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

В doWork () вы получаете значение List из карты потокобезопасным способом. Однако после этого вы получаете доступ к этому списку в небезопасных условиях. Например, один поток может использовать список в doWork () , в то время как другой поток вызывает synchronizedMap.get (ключ) .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
      }
    }
  }

Последний вариант немного ограничит параллелизм, но более понятен IMO.

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

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

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

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

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

10 голосов
/ 16 сентября 2015

Да, вы синхронизируете правильно. Я объясню это более подробно. Вы должны синхронизировать два или более вызовов метода для объекта 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).

4 голосов
/ 19 февраля 2009

Это выглядит правильно для меня. Если бы я что-то изменил, я бы прекратил использовать 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);
2 голосов
/ 01 сентября 2016

Правильный способ синхронизации. Но тут есть подвох

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

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

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

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

б. Его эффективность: d CocurrentMap просто блокирует ключи, следовательно, он не блокирует весь мир карты. Где, поскольку вы заблокировали ключи и значения.

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

2 голосов
/ 19 февраля 2009

Проверить Коллекции Google 'Multimap, например страница 28 из этой презентации .

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

Добро пожаловать на сайт PullRequest, где вы можете задавать вопросы и получать ответы от других членов сообщества.
...