Czy poniższy kod jest skonfigurowany, aby poprawnie zsynchronizować połączenia 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);
}
}
}
}
Z mojego zrozumienia potrzebuję zsynchronizowanego bloku, addToMap()
aby uniemożliwić wywołanie innego wątku remove()
lub containsKey()
zanim przejdę przez wywołanie, put()
ale nie potrzebuję zsynchronizowanego bloku, doWork()
ponieważ inny wątek nie może wejść do zsynchronizowanego bloku addToMap()
przed remove()
powrotem, ponieważ pierwotnie utworzyłem mapę z Collections.synchronizedMap()
. Czy to jest poprawne? Czy jest lepszy sposób, aby to zrobić?
źródło
Collections.synchronizedMap()
? Nie rozumiem drugiego punktu.Jeśli używasz JDK 6, możesz chcieć wypróbować ConcurrentHashMap
Zwróć uwagę na metodę putIfAbsent w tej klasie.
źródło
Istnieje możliwość wystąpienia subtelnego błędu w kodzie.
[ AKTUALIZACJA: Ponieważ używa map.remove (), ten opis nie jest całkowicie poprawny. Brakowało mi tego faktu za pierwszym razem. :( Dziękuję autorowi pytania za wskazanie tego. Resztę zostawiam bez zmian, ale zmieniłem główne stwierdzenie, aby powiedzieć, że jest potencjalnie błąd.]
W doWork () uzyskujesz wartość List z mapy w sposób bezpieczny dla wątków. Później jednak uzyskujesz dostęp do tej listy w niebezpiecznej sprawie. Na przykład, jeden wątek może używać listy w doWork (), podczas gdy inny wątek wywołuje synchronizedMap.get (klucz) .add (wartość) w addToMap () . Te dwa rodzaje dostępu nie są zsynchronizowane. Ogólna zasada jest taka, że gwarancje bezpieczeństwa wątków kolekcji nie obejmują kluczy ani wartości, które przechowują.
Możesz to naprawić, wstawiając zsynchronizowaną listę do mapy, np
List<String> valuesList = new ArrayList<String>(); valuesList.add(value); synchronizedMap.put(key, Collections.synchronizedList(valuesList)); // sync'd list
Alternatywnie możesz zsynchronizować się na mapie podczas korzystania z listy w doWork () :
public void doWork(String key) { List<String> values = null; while ((values = synchronizedMap.remove(key)) != null) { synchronized (synchronizedMap) { //do something with values } } }
Ostatnia opcja nieco ograniczy współbieżność, ale jest nieco bardziej przejrzysta IMO.
Krótka uwaga na temat ConcurrentHashMap. Jest to bardzo przydatna klasa, ale nie zawsze jest odpowiednim zamiennikiem dla zsynchronizowanych HashMaps. Cytując z Javadocs,
Innymi słowy, putIfAbsent () świetnie nadaje się do wstawiania atomowego, ale nie gwarantuje, że inne części mapy nie zmienią się podczas tego wywołania; gwarantuje tylko atomowość. W swoim przykładowym programie polegasz na szczegółach synchronizacji (zsynchronizowanej) mapy HashMap dla rzeczy innych niż put ().
Ostatnia rzecz. :) Ten wspaniały cytat z Java Concurrency in Practice zawsze pomaga mi w projektowaniu debugowania programów wielowątkowych.
źródło
Tak, synchronizujesz poprawnie. Wyjaśnię to bardziej szczegółowo. Musisz zsynchronizować dwa lub więcej wywołań metod w obiekcie synchronizedMap tylko w przypadku, gdy musisz polegać na wynikach poprzedniego wywołania metody w kolejnym wywołaniu metody w sekwencji wywołań metod w obiekcie synchronizedMap. Spójrzmy na ten kod:
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); } }
W tym kodzie
i
wywołania metod opierają się na wyniku poprzedniego
wywołanie metody.
Jeśli sekwencja wywołań metod nie została zsynchronizowana, wynik może być nieprawidłowy. Na przykład
thread 1
wykonuje metodęaddToMap()
ithread 2
wykonuje metodędoWork()
Sekwencja wywołań metod nasynchronizedMap
obiekcie może wyglądać następująco:Thread 1
wykonał metodęa wynik to „
true
”. Po tym, jak system operacyjny przełączył kontrolę wykonywania nathread 2
i wykonałPo tym sterowanie wykonaniem zostało przełączone z powrotem na
thread 1
i na przykład zostało wykonanewierząc, że
synchronizedMap
obiekt zawierakey
iNullPointerException
zostanie wyrzucony, ponieważsynchronizedMap.get(key)
powrócinull
. Jeśli sekwencja wywołań metod nasynchronizedMap
obiekcie nie jest zależna od wzajemnych wyników, nie ma potrzeby synchronizowania sekwencji. Na przykład nie musisz synchronizować tej sekwencji:Tutaj
wywołanie metody nie opiera się na wynikach poprzedniego
wywołanie metody (nie ma znaczenia, czy jakiś wątek ingerował między dwoma wywołaniami metody i na przykład usunął
key1
).źródło
Wydaje mi się to poprawne. Gdybym miał coś zmienić, przestałbym używać Collections.synchronizedMap () i zsynchronizował wszystko w ten sam sposób, tylko po to, aby było jaśniej.
Również wymieniłbym
if (synchronizedMap.containsKey(key)) { synchronizedMap.get(key).add(value); } else { List<String> valuesList = new ArrayList<String>(); valuesList.add(value); synchronizedMap.put(key, valuesList); }
z
List<String> valuesList = synchronziedMap.get(key); if (valuesList == null) { valuesList = new ArrayList<String>(); synchronziedMap.put(key, valuesList); } valuesList.add(value);
źródło
Collections.synchronizedXXX()
interfejsów API, skoro nadal musimy zsynchronizować jakiś obiekt (który w większości przypadków będzie po prostu samą kolekcją) w logice naszej codziennej aplikacjiSposób synchronizacji jest prawidłowy. Ale jest w tym haczyk
Jednak w prawdziwym świecie generalnie przesyła się zapytanie do mapy przed wprowadzeniem wartości. Dlatego musiałbyś wykonać dwie operacje, a zatem potrzebny jest zsynchronizowany blok. Więc sposób, w jaki go użyłeś, jest poprawny. Jednak.
za. Ma API „putIfAbsent”, które robi to samo, ale w bardziej efektywny sposób.
b. Jego wydajność: dThe CocurrentMap po prostu blokuje klucze, dzięki czemu nie blokuje całego świata mapy. Gdzie jak masz zablokowane klucze i wartości.
do. Mogłeś przekazać odniesienie do swojego obiektu mapy gdzie indziej w swojej bazie kodu, gdzie ty / inny programista w twoim teanie może skończyć się nieprawidłowym użyciem go. Tzn. Może po prostu dodawać () lub pobierać () bez blokowania obiektu mapy. Dlatego jego wywołanie nie będzie działało wzajemnie wyłączając się z blokiem synchronizacji. Jednak korzystanie z implementacji współbieżnej daje pewność, że nigdy nie będzie można jej nieprawidłowo użyć / zaimplementować.
źródło
Sprawdź Kolekcje Google "
Multimap
, np stronie 28 tej prezentacji .Jeśli z jakiegoś powodu nie możesz użyć tej biblioteki, rozważ użycie
ConcurrentHashMap
zamiastSynchronizedHashMap
; ma sprytnąputIfAbsent(K,V)
metodę, za pomocą której możesz atomowo dodać listę elementów, jeśli jeszcze jej tam nie ma. Rozważ także użycieCopyOnWriteArrayList
wartości mapy, jeśli pozwalają na to twoje wzorce użytkowania.źródło