¿Cuánta seguridad de hilo es demasiado?

Question

He estado leyendo Java Concurrency in Practice últimamente - gran libro. Si crees que sabes cómo funciona la concurrencia, pero la mayoría de las veces te enfrentas a problemas reales, parece que SWAG es lo máximo que puedes hacer, entonces este libro arrojará algo de luz sobre el tema. Es un poco aterrador cómo muchas cosas realmente pueden salir mal cuando intentas compartir datos entre hilos. Supongo que eso me hizo estar un poco loco por la seguridad de los hilos. Ahora mi preocupación es que, con un poco de sincronización, puedo encontrarme con algunos problemas de vida. He aquí un fragmento de código para ilustrar:

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; 
     } 
    } 

He intentado conseguir que bien: sincronizada colección construido en la sección estática y se almacena en una referencia static final para su publicación segura, bloqueo para la recogida (en lugar de this - de manera que No bloqueo toda la clase en la que vive el código) y el uso de clases de envoltura atómica para primitivos. El libro menciona que exagerar esto también podría causar problemas, pero parece que necesito más tiempo para entenderlo. ¿Cómo harías que este código fuera seguro para subprocesos y asegúrate de que no sufra problemas de vida y de rendimiento?

EDIT: Lo convirtió en métodos de ejemplo y variables, originalmente todo se declaró como estático - mal, mal diseño. También hizo userSessions privada (de alguna manera la dejé pública antes).

Answer 1

Visto desde su código, su sincronización en _userSessions debería bastar porque no expone los objetos AtomicInteger.

La seguridad adicional que ofrece AtomicInteger no es necesaria en este caso, por lo que en esencia la usa aquí como Integer mutable. Puede colocar una clase estática anidada que contenga el recuento de sesión como único atributo en el mapa si le preocupa la sobrecarga adicional en AtomicInteger (o un poco más feo: agregue int [1] al mapa siempre que no lo estén) expuesto fuera de esta clase.)

Answer 2

Use ConcurrentHashMap para que pueda usar putIfAbsent. No necesita el código AtomicInteger para sincronizar.

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; 
    } 

Nota: esto se escapa ...

Intento de una versión no gotea:

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

Buen libro, recientemente he leído yo mismo.

En el código anterior, la única nota que tengo es que AtomicInteger no es necesario dentro del bloque sincronizado, pero dudo que el rendimiento sea notable.

La mejor manera de realizar un seguimiento del rendimiento es probarlo. Configure una prueba de carga integrada automatizada alrededor de las áreas clave de su marco y el rendimiento de la pista. La prueba de carga, si contiene ventanas de tiempo suficientemente amplias y un uso intensivo del flujo de trabajo, también puede atrapar cualquier punto muerto que haya creado.

Si bien los interbloqueos pueden parecer fáciles de evitar, pueden aparecer fácilmente en un patrón de flujo de trabajo bastante simple.

La clase A bloquea los recursos y llama a B (puede ser tan simple como un get/set) que también bloquea el recurso. Otro hilo llama a B que bloquea los recursos y luego llama a A que causa un interbloqueo.

Al trabajar con un marco enriquecido, es útil trazar el flujo de trabajo para ver cómo interactúan las clases. Es posible que pueda detectar este tipo de problema. Sin embargo, con marcos realmente grandes, pueden pasar desapercibidos. La mejor defensa que he encontrado es aislar bloqueos en el área más pequeña posible y ser muy consciente de una llamada fuera de una clase mientras está dentro de un bloque sincronizado. Crear un número significativo de pruebas de carga ayuda también.

Answer 4

Antes que nada: ¡No use la Hashtable! Es viejo y muy lento.
Adicional: la sincronización en el nivel inferior no es necesaria si ya sincroniza en un nivel superior (Esto también se aplica a AtomicInteger-thing).

Veo diferentes enfoques aquí, de acuerdo con el caso de uso que se necesita aquí.

La lectura/escritura enfoque

Suponiendo que se llama al método isUserRegistered muy a menudo y los demás métodos sólo de vez en cuando, una buena manera es una lectura-escritura-bloqueo: Se les permite tener múltiples lee al mismo tiempo, pero solo un bloqueo de escritura para controlarlos a todos (solo se puede obtener si no se requiere otro bloqueo).

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: sencillo de entender
En contra: no se escala si los métodos de escritura son llamados con frecuencia

La pequeña atómica operación de aproximación

La idea es hacer pequeños pasos , que son todos atómicos. Esto conducirá a un rendimiento muy bueno de todos modos, pero hay muchas trampas ocultas aquí.

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: escalas muy bien
En contra: No intuitiva, va a ser compleja rápidamente, no siempre es posible, muchas trampas ocultas

El bloqueo por aproximación de usuario

Esta voluntad cree bloqueos para diferentes usuarios, suponiendo que haya muchos usuarios diferentes. Puede crear bloqueos o monitores con algunas operaciones atómicas pequeñas y bloquearlos en lugar de la lista completa.
Sería excesivo para este pequeño ejemplo, pero para estructuras muy complejas puede ser una solución elegante.

Answer 5

Me encontré con esta pregunta ancestral en busca de consejos sobre cómo hacer lo que uno podría llamar un "mapa de conteo concurrente" - buscando en particular para los usos de ConcurrentHashMap con AtomicInteger.

Aquí hay una versión modificada de the highest-rated answer que usa AtomicInteger y no tiene fugas. En mi (limitada) prueba esto parece mucho más rápido que la versión Integer. También notaré que usar ConcurrentMap.get() antes de ConcurrentMap.putIfAbsent() parece ahorrar un tiempo notable.

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; 
    } 
} 

velocidad puede no ser tan relevante para el problema de su creador original, pero otras aplicaciones de una "hoja de recuento" de este tipo puede beneficiarse de la eficiencia.