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

85

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

Райан Ахерн
источник

Ответы:

90

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

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

Юваль Адам
источник
26
Я думаю, было бы хорошо упомянуть, что это работает, потому что в javadocs явно указано, что synchronizedMap синхронизируется на самой карте, а не на какой-то внутренней блокировке. Если бы это было так, то synchronized (synchronizedMap) не был бы правильным.
extraneon
2
@Yuval, не могли бы вы объяснить свой ответ более подробно? Вы говорите, что sychronizedMap выполняет операции атомарно, но тогда зачем вам вообще нужен собственный синхронизированный блок, если syncMap делает все ваши операции атомарными? Ваш первый абзац, кажется, избавляет от беспокойства по поводу второго.
almel
@almel посмотри мой ответ
Сергей
2
зачем нужен синхронизированный блок, который уже используется на карте Collections.synchronizedMap()? Я не понимаю второго пункта.
Бимал Шарма
15

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

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

Тофу пиво
источник
9
На самом деле это JDK 1.5
Богдан
13

В вашем коде может быть небольшая ошибка.

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

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

Последний вариант немного ограничит параллелизм, но ИМО несколько яснее.

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

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

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

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

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

JLR
источник
Я понимаю вашу точку зрения об ошибке, если бы я обращался к списку с помощью synchronizedMap.get (). Поскольку я использую remove (), не должно ли следующее добавление с этим ключом создавать новый список ArrayList и не мешать тому, который я использую в doWork?
Райан Ахерн,
Верный! Я полностью пролетел мимо твоего удаления.
JLR
1
Для каждой изменяемой переменной состояния, к которой может получить доступ более одного потока, все обращения к этой переменной должны выполняться с одной и той же удерживаемой блокировкой. ---- Я обычно добавляю частное свойство, которое представляет собой просто новый объект (), и использую его для своих блоков синхронизации. Таким образом, я знаю все это насквозь в этом контексте. synchronized (objectInVar) {}
AnthonyJClink
11

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

Мне это кажется правильным. Если бы мне пришлось что-то изменить, я бы перестал использовать 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);
Пол Томблин
источник
3
Что нужно сделать. Я не понимаю, почему мы должны использовать Collections.synchronizedXXX()API, когда нам все еще нужно синхронизировать какой-то объект (который в большинстве случаев будет просто самой коллекцией) в логике нашего повседневного приложения
kellogs
3

Вы правильно синхронизировали. Но есть загвоздка

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

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

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

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

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

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

Джай Пандит
источник
2

Проверьте Google Collections " Multimap, например , страница 28 из презентации .

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

Баренд
источник