Sincronización en un caché HashMap

Question

Tengo una aplicación web donde las personas piden recursos. Estos recursos se almacenan en caché utilizando un mapa hash sincronizado para mayor eficiencia. El problema aquí es cuando llegan dos solicitudes diferentes para el mismo recurso no almacenado al mismo tiempo: la operación que recupera los recursos consume mucha memoria, por lo que quiero evitar llamarla más de una vez por el mismo recurso.

¿Alguien puede decirme si hay algún problema potencial con el siguiente fragmento? Gracias por adelantado.

private Map<String, Resource> resources = Collections.synchronizedMap(new HashMap<String, Resource>()); 

public void request(String name) { 

    Resource resource = resources.get(name); 

    if (resource == null) { 
    synchronized(this) { 
     if (resources.get(name) == null) { 
     resource = veryCostlyOperation(name); // This should only be invoked once per resource... 
     resources.put(resource); 
     } else { 
     resource = resources.get(name); 
     } 
    } 
    } 

    ... 

} 

Answer 1

Un posible problema es que se crea la discordia innecesaria mediante la ejecución de veryCostlyOperation() dentro de un bloque synchronized, por lo que muchos hilos no pueden recuperar sus recursos (independientes) al mismo tiempo. Esto se puede resolver mediante el uso de Future<Resource> como valores del mapa:

Map<String, Future<Resource>> map = new ConcurrentHashMap<String, Future<Resource>>();  
... 
Future<Resource> r = map.get(name); 
if (r == null) { 
    FutureTask task = null; 
    synchronized (lock) { 
     r = map.get(name); 
     if (r == null) { 
      task = new FutureTask(new Callable<Resource>() { 
       public Resource call() { 
        return veryCostlyOperation(name); 
       } 
      }); 
      r = task; 
      map.put(name, r); 
     } 
    } 
    if (task != null) task.run(); // Retrieve the resource 
} 

return r.get(); // Wait while other thread is retrieving the resource if necessary 

Answer 2

El único problema potencial que veo es que sincronice a this. Si cualquier otro código en la misma clase también se sincroniza con this, solo uno de esos bloques se ejecutará a la vez. Tal vez no hay nada más que haga esto, y está bien. Sin embargo, siempre me preocupa lo que el próximo programador va a hacer. (o yo mismo en tres meses cuando me olvidé de este código)

Recomendaría crear un objeto de sincronización genérico y luego sincronizar con eso.

 
private final Object resourceCreationSynchObject = new Object(); 

continuación

 
synchronized(this.resourceCreationSynchObject) { 
    ... 
} 

De lo contrario, esto es exactamente lo que estás pidiendo. Asegura que no se puede llamar al veryCostlyOperation en paralelo.

Además, es genial pensar en volver a obtener el recurso por segunda vez dentro del bloque synchronized. Esto es necesario, y la primera llamada externa asegura que no se sincronice cuando el recurso ya está disponible. Pero no hay razón para llamarlo por tercera vez. Primero dentro del bloque synchronized, configure resource nuevamente en resources.get(name) y luego verifique esa variable para nulo. Eso evitará que tengas que volver a llamar al get dentro de la cláusula else.

Answer 3

Su código se ve bien, excepto que está sincronizando más de que realmente se requiere:

• El uso de un ConcurrentHashMap en lugar de un sincronizado HashMap permitiría múltiples invocaciones del método get sin bloquear.

• Sincronizando en this en lugar de resources probablemente no sea necesario, pero depende del resto de su código.

Answer 4

Su código potencialmente llamar veryCostlyOperation (nombre) varias veces. El problema es que hay un paso no sincronizado después de buscar el mapa:

public void request(String name) { 
    Resource resource = resources.get(name); 
    if (resource == null) { 
     synchronized(this) { 
      //... 
     } 
    } 
    //... 
} 

el get() del mapa está sincronizado por el mapa, pero comprobar el resultado de nulo no está protegido por nada.Si varios subprocesos entran en este solicitando el mismo "nombre", todos verán un resultado nulo de resources.get(), hasta que realmente termine costoso Operación y lo coloque en el mapa de recursos.

Un enfoque más simple y funcional, pero menos escalable, sería ir con un mapa normal y sincronizar todo el método de solicitud. A menos que realmente resulte un problema en la práctica, elegiría el enfoque simple.

Para una mayor escalabilidad puede corregir su código verificando el mapa nuevamente después de sincronizado (esto) para atrapar el caso descrito anteriormente. Todavía no daría la mejor escalabilidad, ya que el sincronizado (esto) solo permite que un subproceso ejecute costosOperación, mientras que en muchos casos prácticos, solo desea evitar múltiples ejecuciones para el mismo recurso mientras permite solicitudes concurrentes al diferente recursos. En ese caso, necesita alguna facilidad para sincronizarse en el recurso que se solicita. Un ejemplo muy básico:

private static class ResourceEntry { 
    public Resource resource; 
} 

private Map<String, ResourceEntry> resources = new HashMap<String, ResourceEntry>(); 

public Resource request(String name) { 
    ResourceEntry entry; 
    synchronized (resources) { 
     entry = resources.get(name); 
     if (entry == null) { 
      // if no entry exists, allocate one and add it to map 
      entry = new ResourceEntry(); 
      resources.put(name, entry); 
     } 
    } 
    // at this point we have a ResourceEntry, but it *may* be no loaded yet 
    synchronized (entry) { 
     Resource resource = entry.resource; 
     if (resource == null) { 
      // must create the resource 
      resource = costlyOperation(name); 
      entry.resource = resource; 
     } 
     return resource; 
    } 
} 

Este es solo un boceto. Básicamente, realiza una búsqueda sincrónica de ResourceEntry y luego se sincroniza en ResourceEntry para garantizar que el recurso específico solo se genere una vez.