Question

J'ai une application Web où les gens demandent des ressources. Ces ressources sont mises en cache à l'aide d'une carte de hachage synchronisée pour l'efficacité. Le problème ici est de savoir quand deux demandes différentes viennent pour la même ressource non achetée en même temps: l'opération récupérant les ressources prend beaucoup de mémoire, donc je veux éviter de l'appeler plus d'une fois pour la même ressource.

Quelqu'un peut-il me dire s'il y a un problème potentiel avec l'extrait suivant? Merci d'avance.

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

  ...

}
Était-ce utile?

La solution

Un problème possible est que vous créez une affirmation inutile en exécutant veryCostlyOperation() à l'intérieur d'un synchronized Block, de sorte que de nombreux threads ne peuvent pas récupérer leurs ressources (indépendantes) en même temps. Cela peut être résolu en utilisant Future<Resource> Comme valeurs de la carte:

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

Autres conseils

Le seul problème potentiel que je vois est que vous vous synchronisez this. Si un autre code de la même classe se synchronise également this, un seul de ces blocs fonctionnera en même temps. Peut-être qu'il n'y a rien d'autre qui fait ça, et ça va. Je m'inquiète toujours de ce que le prochain programmeur va faire, cependant. (ou moi-même dans trois mois quand j'ai oublié ce code)

Je recommanderais de créer un objet de synchronisation générique puis de synchroniser avec cela.

private final Object resourceCreationSynchObject = new Object();

alors

synchronized(this.resourceCreationSynchObject) {
  ...
}

Sinon, cela fait exactement ce que vous demandez. Il garantit que veryCostlyOperation ne peut pas être appelé en parallèle.

En outre, c'est une grande réflexion pour recueillir la ressource une deuxième fois dans le synchronized bloquer. Cela est nécessaire et le premier appel à l'extérieur s'assure que vous ne vous synchronisez pas lorsque la ressource est déjà disponible. Mais il n'y a aucune raison de l'appeler une troisième fois. Première chose à l'intérieur du synchronized Bloquer, régler resource à nouveau resources.get(name) puis vérifiez cette variable pour NULL. Qui vous empêchera d'avoir à appeler get encore à l'intérieur du else clause.

Votre code semble correct, sauf que vous synchronisez plus que réellement requis:

  • Utilisant un ConcurrentHashMap au lieu d'un synchronisé HashMap permettrait de plusieurs invocations de la méthode GET sans verrouillage.

  • Synchroniser this à la place de resources n'est probablement pas nécessaire, mais cela dépend du reste de votre code.

Votre code appellera potentiellement plusieurs fois de manière très opérationnelle (nom). Le problème est qu'il y a une étape non synchronisée après avoir recherché la carte:

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

Le get () de la carte est synchronisé par la carte, mais la vérification du résultat pour NULL n'est protégé par rien. Si plusieurs threads entrent dans cette demande le même "nom", tous verront un résultat nul de Resources.get (), jusqu'à ce que l'on termine réellement laOpection Costly et met la ressource dans la carte des ressources.

Une approche plus simple et fonctionnelle, mais moins évolutive serait d'aller avec une carte normale et de faire synchronisé la méthode de demande entière. À moins que cela ne se produise réellement un problème dans la pratique, je choisirais l'approche simple.

Pour une évolutivité plus élevée, vous pouvez corriger votre code en vérifiant la carte encore après synchronisé (ceci) pour attraper le cas décrit ci-dessus. Il ne donnerait toujours pas la meilleure évolutivité, car le synchronisé (ceci) ne permet qu'à un thread d'exécuter une opération coûteuse, tandis que dans de nombreux cas pratiques, vous voulez uniquement empêcher plusieurs exécutions pour le même ressource tout en permettant aux demandes simultanées de différent Ressources. Dans ce cas, vous avez besoin d'une installation pour vous synchroniser sur la ressource demandée. Un exemple très basique:

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

Ce n'est qu'un croquis approximatif. Fondamentalement, il fait une recherche sychronisée pour un ressourcement, et alors Synchronise sur le ressourceenture pour s'assurer que la ressource spécifique est uniquement construite une fois que.

Licencié sous: CC-BY-SA avec attribution
Non affilié à StackOverflow
scroll top