Настроен ли следующий код для правильной синхронизации вызовов 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()
. Это верно? Есть лучший способ сделать это?
источник
Collections.synchronizedMap()
? Я не понимаю второго пункта.Если вы используете JDK 6, вы можете проверить ConcurrentHashMap
Обратите внимание на метод putIfAbsent в этом классе.
источник
В вашем коде может быть небольшая ошибка.
[ ОБНОВЛЕНИЕ: поскольку он использует 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,
Другими словами, putIfAbsent () отлично подходит для атомарных вставок, но не гарантирует, что другие части карты не изменятся во время этого вызова; он гарантирует только атомарность. В вашем примере программы вы полагаетесь на детали синхронизации (синхронизированного) HashMap для других вещей, кроме put () s.
Последнее. :) Эта замечательная цитата из Java Concurrency in Practice всегда помогает мне в разработке отладочных многопоточных программ.
источник
Да, вы правильно синхронизируете. Я объясню это более подробно. Вы должны синхронизировать два или более вызова методов для объекта 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); } }
В этом коде
а также
вызовы методов полагаются на результат предыдущего
вызов метода.
Если последовательность вызовов методов не была синхронизирована, результат может быть неверным. Например
thread 1
, выполняется методaddToMap()
иthread 2
выполняется методdoWork()
. Последовательность вызовов метода дляsynchronizedMap
объекта может быть следующей:Thread 1
выполнил методи результат "
true
". После этого операционная система переключила управление выполнением наthread 2
и выполнилаПосле этого управление выполнением было переключено обратно на,
thread 1
и он, например, выполнилполагая, что
synchronizedMap
объект содержитkey
иNullPointerException
будет брошен, потомуsynchronizedMap.get(key)
что вернетсяnull
. Если последовательность вызовов методов дляsynchronizedMap
объекта не зависит от результатов друг друга, вам не нужно синхронизировать последовательность. Например, вам не нужно синхронизировать эту последовательность:Вот
вызов метода не полагается на результаты предыдущего
вызов метода (его не волнует, вмешался ли какой-либо поток между двумя вызовами метода и, например, удалил
key1
).источник
Мне это кажется правильным. Если бы мне пришлось что-то изменить, я бы перестал использовать 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);
источник
Collections.synchronizedXXX()
API, когда нам все еще нужно синхронизировать какой-то объект (который в большинстве случаев будет просто самой коллекцией) в логике нашего повседневного приложенияВы правильно синхронизировали. Но есть загвоздка
Однако в реальном мире вы обычно запрашиваете карту, прежде чем вводить значение. Следовательно, вам нужно будет выполнить две операции и, следовательно, потребуется синхронизированный блок. Так что способ, которым вы его использовали, правильный. Однако.
а. У него есть API putIfAbsent, который будет делать то же самое, но более эффективно.
б. Его эффективность: d CocurrentMap просто блокирует ключи, следовательно, он не блокирует весь мир карты. Где, как вы заблокировали ключи, а также значения.
c. Вы могли передать ссылку на свой объект карты где-нибудь еще в своей кодовой базе, где вы / другой разработчик в вашем tean можете в конечном итоге использовать ее неправильно. Т.е. он может просто добавить () или получить () без блокировки объекта карты. Следовательно, его вызов не будет взаимоисключающим с вашим блоком синхронизации. Но использование параллельной реализации дает вам уверенность в том, что она никогда не может быть использована / реализована неправильно.
источник
Проверьте Google Collections "
Multimap
, например , страница 28 из презентации .Если по какой-то причине вы не можете использовать эту библиотеку, рассмотрите возможность использования
ConcurrentHashMap
вместоSynchronizedHashMap
; у него есть изящныйputIfAbsent(K,V)
метод, с помощью которого вы можете атомарно добавить список элементов, если его еще нет. Кроме того, рассмотрите возможность использованияCopyOnWriteArrayList
значений карты, если это оправдано вашими шаблонами использования.источник