🚀 go-pugleaf

On 2014-05-30 19:21, tomjas wrote:
> Witam.
>
> Tak w ramach ćwiczeń postanowiłem sobie poćwiczyć różne wersje wzorca
> singleton. Poniżej kod testujący singleton w najprostszej wersji
> Lazy-Initialization (oczywiście abstrahujemy od jakości wersji wzorca
> - wiem, że nie jest to najlepsze rozwiązanie). A więc sam signleton:
>
> #v+
> class Counter {
>      private static Counter INSTANCE;
>
>      private Counter(){}
>
>      public static synchronized Counter getInstance() {
>          if (INSTANCE == null) {
>              INSTANCE = new Counter();
>          }
>          return INSTANCE;
>      }
> }

To nie jest najwydajniejsza implementacja singletona z lazy initem. 
Najlepsza w Javie jest oparta na wewnętrznej klasie.

public class Something {
     private Something() {}

     private static class LazyHolder {
         private static final Something INSTANCE = new Something();
     }

     public static Something getInstance() {
         return LazyHolder.INSTANCE;
     }
}

Ten pattern gwarantuje leniwą inicjalizację, i nie ma straty wydajnosći 
na synchronizowaniu.

>
> teraz klasa, która zawiera kod w tworzonych wątkach. Do konstruktora
> przekazuję hashmapę, w której będę zliczał obiekty o takim samym
> hashkodzie:
>
>
> #v+
> class CounterThread implements Runnable {
>
>      Map<Integer, Integer> map;
>
>      public CounterThread(Map<Integer, Integer> map) {
>          this.map = map;
>      }
>
>      @Override
>      public void run() {
>          Counter cnt = Counter.getInstance();
>          int hash = cnt.hashCode();

>          Integer freq = map.get(hash);
>          map.put(hash, (freq == null) ? 1 : freq + 1);

Te dwie linie to popularny błąd wiary w cudowne właściwości 
Collections.synchronizedMap().
Pomiędzy map.get(hash) a map.put(hash, ..) inny wątek może zmienić mapę, 
i dane lecą w kosmos (czyli któryś z wątków zapisze do mapy wartość 
która już tam jest, choć wydaje mu się że ją zwiększył).

Żeby było poprawnie i bezpiecznie to te dwie linie należy 
zsynchronizować na mapie, np tak:

class CounterThread implements Runnable {

     Map<Integer, Integer> map;

     public CounterThread(Map<Integer, Integer> map) {
         this.map = map;
     }

     @Override
     public void run() {
         Counter cnt = Counter.getInstance();
         int hash = cnt.hashCode();
         synchronized(map) {
           Integer freq = map.get(hash);
           map.put(hash, (freq == null) ? 1 : freq + 1);
         }
     }
}

... i w takim wypadku mapa nie musi być już przepiszczana poprzez 
Collections.synchronizedMap().

Albo użyj synchronizowanej mapy, w wersji która dla get nieznanego 
klucza automatycznie tworzy parę klucz/wartość, a jako wartość użyj 
AtomicInteger.
Kod wywołania wyglądał by wtedy tak:

     public void run() {
         Counter cnt = Counter.getInstance();
         int hash = cnt.hashCode();
	map.get(hash).incrementAndGet();
     }



>      }
> }
> #v-
>
> No i klasa tworząca 1000 wątków.
>
> #v+
> public class Main {
>      public static void main(String[] args) {
>
>          Map<Integer, Integer> map = Collections
>                  .synchronizedMap(new HashMap<Integer, Integer>());
>
>          for (int i = 0; i < 1_000; i++) {
>              new Thread(new CounterThread(map)).start();
>          }
>
>          try {
>              Thread.sleep(5000);
>              System.out.println(map);
>          } catch (InterruptedException e) {
>          }
>      }
> }
> #v-
>
>
> Dla pewności daję głównegmu wątkowi odpocząć 5 sek i sprawdzam hashmapę.
> A wynik wygląda np. tak:
>
> {1704732977=897}
>
>
> Rzecz jasna za każdym wywołaniem zmienia się wartość hashkodu i ich
> liczba, ale - i tu moje pytanie - co z resztą wątków tworzących obiekty
> singletona (Counter) i licząca dla nich haskody? Przecież powinno być
> ich 1000. Gdzie popełniam błąd, bo już mi monitor oczy wypala
> od wpatrywania?
>

Z powodu błędów i wyścigu wątków nie każdy wątek zdołał zwiększyć licznik.

> Dodatkowo, dlaczego jeśli utworzę zwykłą hashmapę bez synchronizacji, to
> jej odczyt wygląda jak powyżej, a odczyt jej wielkości (map.size())
> potrafi pokazać 2-5 elementów?

Bo zwykła mapa bez synchronizacji nie jest bezpieczna przy korzystaniu 
wielowątkowym, i cuda się dzieją => zachowanie niezdefiniowane.

-- 
Arivald