Сколько потоковой безопасности слишком много?

Question

Я читал Java Concurrency in Practice последнее время - отличная книга. Если вы считаете, что знаете, как работает параллелизм, но в большинстве случаев вы сталкиваетесь с реальными проблемами, похоже, что SWAG - это самое лучшее, что вы можете сделать, то эта книга, несомненно, прольет некоторый свет на эту тему. Это страшно, как многие вещи могут ошибиться при попытке обмена данными между потоками. Думаю, это заставило меня, наверное, немного сумасшедшим в отношении безопасности потоков. Теперь я обеспокоен тем, что с чересчур большой синхронизацией я могу столкнуться с некоторыми проблемами с жизнеспособностью. Вот кусок кода, чтобы проиллюстрировать:

private final Hashtable<String, AtomicInteger> userSessions = 
new Hashtable<String, AtomicInteger>(); 

    public void registerUser(String userLogin) { 
     synchronized(userSessions) { 
      AtomicInteger sessionCount = userSessions.get(userLogin); 
      if (sessionCount != null) { 
       sessionCount.incrementAndGet(); 
      } else { 
       userSessions.put(userLogin, new AtomicInteger(1)); 
      } 
     } 
    } 

    public void unregisterUser(String userLogin) { 
     synchronized(userSessions) { 
      AtomicInteger sessionCount = userSessions.get(userLogin); 
      if (sessionCount != null) { 
       sessionCount.decrementAndGet(); 
      } 
     } 
    } 

    public boolean isUserRegistered(String userLogin) { 
     synchronized(userSessions) { 
      AtomicInteger sessionCount = userSessions.get(userLogin); 
      if (sessionCount == null) { 
       return false; 
      } 
      return sessionCount.intValue() > 0; 
     } 
    } 

Я пытался получить его все в порядке: синхронизировано коллекция построена в статическом разделе и хранится в статической окончательной ссылке для безопасной публикации, запирая на коллекции (вместо this - так что Я не блокирую весь класс, в котором живет код) и использует классы атомных оболочек для примитивов. В книге упоминается, что переусердство может также вызвать проблемы, но мне кажется, мне нужно больше времени, чтобы полностью обернуть вокруг себя голову. Как вы можете сделать этот код потокобезопасным и убедиться, что он не страдает от живучести, а также проблем с производительностью?

EDIT: превратили его в экземпляры методов и переменных, изначально все было объявлено как статическое - плохой, плохой дизайн. Также сделал userSessions частным (как-то я оставил его публичным раньше).

Answer 1

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

Дополнительная безопасность, предлагаемая AtomicInteger, в этом случае не требуется, поэтому в основном вы используете ее здесь как изменчивый Integer. Вы можете поместить вложенный статический класс, содержащий счет сеанса, только как атрибут на карте, если вы беспокоитесь о дополнительных накладных расходах в AtomicInteger (или немного уродливее: добавьте int [1] на карту, если они не являются выставлены вне этого класса.)

Answer 2

Используйте ConcurrentHashMap, чтобы вы могли использовать putIfAbsent. Вам не нужно кодировать код AtomicInteger.

public final ConcurrentMap<String, AtomicInteger> userSessions = 
     new ConcurrentHashMap<String, AtomicInteger>(); 

    public void registerUser(String userLogin) { 
     AtomicInteger newCount = new AtomicInteger(1); 
     AtomicInteger oldCount = userSessions.putIfAbsent(userLogin, newCount); 
     if (oldCount != null) { 
      oldCount.incrementAndGet(); 
     } 
    } 

    public void unregisterUser(String userLogin) { 
     AtomicInteger sessionCount = userSessions.get(userLogin); 
     if (sessionCount != null) { 
      sessionCount.decrementAndGet(); 
     } 
    } 

    public boolean isUserRegistered(String userLogin) { 
     AtomicInteger sessionCount = userSessions.get(userLogin); 
     return sessionCount != null && sessionCount.intValue() > 0; 
    } 

Заметим, что это утечка ...

Попытка не-вытекающей версии:

public final ConcurrentMap<String, Integer> userSessions = 
     new ConcurrentHashMap<String, Integer>(); 

    public void registerUser(String userLogin) { 
     for (;;) { 
      Integer old = userSessions.get(userLogin); 
      if (userSessions.replace(userLogin, old, old==null ? 1 : (old+1)) { 
       break; 
      } 
     } 
    } 
    public void unregisterUser(String userLogin) { 
     for (;;) { 
      Integer old = userSessions.get(userLogin); 
      if (old == null) { 
       // Wasn't registered - nothing to do. 
       break; 
      } else if (old == 1) { 
       // Last one - attempt removal. 
       if (userSessions.remove(userLogin, old)) { 
        break; 
       } 
      } else { 
       // Many - attempt decrement. 
       if (userSessions.replace(userLogin, old, old-1) { 
        break; 
       } 
      } 
     } 
    } 
    public boolean isUserRegistered(String userLogin) {serLogin); 
     return userSessions.containsKey(userLogin); 
    } 

Answer 3

Хорошая книга, я недавно прочитал сам.

В приведенном выше коде единственное примечание, которое у меня есть, это то, что AtomicInteger не требуется в синхронизированном блоке, но я сомневаюсь, что производительность будет заметной.

Лучший способ отслеживания производительности - проверить его. Настройте автоматизированный интегрированный нагрузочный тест вокруг ключевых областей вашей инфраструктуры и производительности. Тест нагрузки, если он содержит достаточно широкие временные окна и богатое использование рабочего потока, также может захватывать любые взаимоблокировки, которые вы создали.

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

Класс A блокирует ресурсы, а затем вызывает B (может быть так же просто, как get/set), который также блокирует ресурс. Другой поток вызывает B, который блокирует ресурсы, а затем вызывает A, вызывающий тупик.

При работе с богатым фреймворком полезно отобразить рабочий процесс, чтобы увидеть, как взаимодействуют классы. Вы можете обнаружить эту проблему. Однако, с действительно большими каркасами, они могут проскользнуть. Лучшая защита, которую я обнаружил, заключается в том, чтобы изолировать блокировки до наименьшей возможной области и очень хорошо понимать вызов за пределами класса, находясь в пределах синхронизированного блока. Также помогает создать значительное количество нагрузочных тестов.

Answer 4

Прежде всего: не используйте Hashtable! Он старый и очень медленный.
Дополнительно: Синхронизация на нижнем уровне не требуется, если вы уже синхронизированы на более высоком уровне (это верно и для AtomicInteger-thing).

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

чтения/записи подход

Предполагая, что вы называете isUserRegistered метод очень часто и другие методы только сейчас, а затем, хороший способ для чтения-записи блокировки: Разрешается иметь несколько читает в то же время, но только один замок записи, чтобы управлять ими всеми (может быть получен только в том случае, если никакая другая блокировка не будет получена).

private static final Map<String, Integer> _userSessions = 
    new HashMap<String, Integer>(); 

private ReadWriteLock rwLock = 
    new ReentrantReadWriteLock(false); //true for fair locks 

public static void registerUser(String userLogin) { 
    Lock write = rwLock.writeLock(); 
    write.lock(); 
    try { 
     Integer sessionCount = _userSessions.get(userLogin); 
     if (sessionCount != null) { 
      sessionCount = Integer.valueOf(sessionCount.inValue()+1); 
     } else { 
      sessionCount = Integer.valueOf(1) 
     } 
     _userSessions.put(userLogin, sessionCount); 
    } finally { 
    write.unlock(); 
    } 
} 

public static void unregisterUser(String userLogin) { 
    Lock write = rwLock.writeLock(); 
    write.lock(); 
    try { 
     Integer sessionCount = _userSessions.get(userLogin); 
     if (sessionCount != null) { 
      sessionCount = Integer.valueOf(sessionCount.inValue()-1); 
     } else { 
      sessionCount = Integer.valueOf(0) 
     } 
     _userSessions.put(userLogin, sessionCount); 
    } finally { 
    write.unlock(); 
    } 
} 

public static boolean isUserRegistered(String userLogin) { 
    boolean result; 

    Lock read = rwLock.readLock(); 
    read.lock(); 
    try { 
     Integer sessionCount = _userSessions.get(userLogin); 
     if (sessionCount != null) { 
      result = sessionCount.intValue()>0 
     } else { 
      result = false; 
     } 
    } finally { 
    read.unlock(); 
    } 

    return false; 
} 

Pro: просто понять
Con: не будет масштабироваться, если методы записи называются часто

Небольшой атомарных операций подход

Идея заключается в том, чтобы делать небольшие шаги , которые являются атомарными. В любом случае это приведет к очень хорошей производительности, но здесь есть много скрытых ловушек.

public final ConcurrentMap<String, AtomicInteger> userSessions = 
    new ConcurrentHashMap<String, AtomicInteger>(); 
//There are other concurrent Maps for different use cases 

public void registerUser(String userLogin) { 
    AtomicInteger count; 
    if (!userSession.containsKey(userLogin)){ 
    AtomicInteger newCount = new AtomicInteger(0); 
    count = userSessions.putIfAbsent(userLogin, newCount); 
    if (count == null){ 
     count=newCount; 
    } 
    //We need ifAbsent here, because another thread could have added it in the meantime 
    } else { 
    count = userSessions.get(userLogin); 
    } 
    count.incrementAndGet(); 
} 

public void unregisterUser(String userLogin) { 
    AtomicInteger sessionCount = userSessions.get(userLogin); 
    if (sessionCount != null) { 
    sessionCount.decrementAndGet(); 
    } 
} 

public boolean isUserRegistered(String userLogin) { 
    AtomicInteger sessionCount = userSessions.get(userLogin); 
    return sessionCount != null && sessionCount.intValue() > 0; 
} 

Pro: весы очень хорошо
Con: Не интуитивным, будет сложным быстро, не всегда возможно, много скрытых ловушек

Замок на пользователя подход

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

Answer 5

Я столкнулся с этим многолетним вопросом, ища совет о том, как сделать то, что можно назвать «параллельной подсчетной картой» - поиск, в частности, для использования ConcurrentHashMap с AtomicInteger.

Вот модифицированная версия the highest-rated answer, которая использует AtomicInteger и не течет. В моем (ограниченном) тестировании это кажется намного быстрее, чем версия Integer. Я также отмечу, что использование ConcurrentMap.get() до ConcurrentMap.putIfAbsent(), похоже, экономит время.

private final ConcurrentMap<String, AtomicInteger> userSessions = 
    new ConcurrentHashMap<String, AtomicInteger>(); 

public void registerUser(String userLogin) { 
    AtomicInteger oldCount = userSessions.get(key); 
    if(oldCount!=null && getAndIncrementIfNonZero(oldCount)>0) return; 
    AtomicInteger newCount = new AtomicInteger(1); 
    while(true) { 
     oldCount = userSessions.putIfAbsent(key, newCount); 
     if(oldCount==null) return; 
     if(getAndIncrementIfNonZero(oldCount)>0) return; 
    } 
} 

public void unregisterUser(String userLogin) { 
    AtomicInteger sessionCount = userSessions.get(userLogin); 
    if (sessionCount != null) { 
     int endingCount = sessionCount.decrementAndGet(); 
     if(endingCount==0) userSessions.remove(userLogin); 
    } 
} 

public boolean isUserRegistered(String userLogin) { 
    AtomicInteger sessionCount = userSessions.get(userLogin); 
    return sessionCount != null && sessionCount.intValue() > 0; 
} 

private static int getAndIncrementIfNonZero(AtomicInteger ai) { 
    while(true) { 
     int current = ai.get(); 
     if(current==0) return 0; 
     if(ai.compareAndSet(current, current+1)) return current; 
    } 
} 

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