Gardez-vous une fermeture «évidente» ou utilisez-vous un verrouillage à double contrôle?

Question

Je crains de formuler des questions. J'ai le code morceau de (Java) suivante (pseudo):

public SomeObject getObject(Identifier someIdentifier) { 
    // getUniqueIdentifier retrieves a singleton instance of the identifier object, 
    // to prevent two Identifiers that are equals() but not == (reference equals) in the system. 
    Identifier singletonInstance = getUniqueIdentifier(someIdentifier); 
    synchronized (singletonInstance) { 
     SomeObject cached = cache.get(singletonInstance); 
     if (cached != null) { 
      return cached; 
     } else { 
      SomeObject newInstance = createSomeObject(singletonInstance); 
      cache.put(singletonInstance, newInstance); 
      return newInstance; 
     } 
    } 
} 

Fondamentalement, il est 'unique' un identifiant (référence est égal, comme dans ==), vérifie un cache, et dans le cas d'un défaut de cache , appelle une méthode coûteuse (impliquant l'appel d'une ressource externe et l'analyse, etc), met cela dans le cache, et retourne. Le Identifier synchronisé, dans ce cas, évite deux equals() mais pas ==Identifier objets utilisés pour appeler la méthode coûteuse, qui permettrait de récupérer la même ressource simultanément.

Les travaux ci-dessus. Je me demandais juste, et probablement micro-optimisant, une réécriture telle que la suivante qui utilise plus de récupération de cache naïf et double-vérifier le verrouillage soit «sûr» (sûr comme dans threadsafe, vide de conditions de course impair) et être 'plus optimal '(comme dans une réduction des verrous inutiles et des threads devant attendre un verrou)?

public SomeObject getObject(Identifier someIdentifier) { 

    // just check the cache, reference equality is not relevant just yet. 
    SomeObject cached = cache.get(someIdentifier); 
    if (cached != null) { 
     return cached; 
    }   

    Identifier singletonInstance = getUniqueIdentifier(someIdentifier); 
    synchronized (singletonInstance) { 
     // re-check the cache here, in case of a context switch in between the 
     // cache check and the opening of the synchronized block. 
     SomeObject cached = cache.get(singletonInstance); 
     if (cached != null) { 
      return cached; 
     } else { 
      SomeObject newInstance = createSomeObject(singletonInstance); 
      cache.put(singletonInstance, newInstance); 
      return newInstance; 
     } 
    } 
} 

On pourrait dire « juste le tester » ou « Il suffit de faire un micro-référence », mais de tester les bits multi-thread de code n'est pas mon point fort, et je doute que je serais capable de simuler situations réalistes ou fausses conditions de course avec précision. De plus, cela me prendrait une demi-journée, alors que l'écriture d'une question SO ne me prend que quelques minutes :).

Answer 1

Vous réinventent Google-Collections/Goyave de MapMaker/ComputingMap:

ConcurrentMap<Identifier, SomeObject> cache = new MapMaker().makeComputingMap(new Function<Identifier, SomeObject>() { 
    public SomeObject apply(Identifier from) { 
    return createSomeObject(from); 
    } 
}; 

public SomeObject getObject(Identifier someIdentifier) { 
    return cache.get(someIdentifier); 
} 

interner n'est pas nécessaire ici que le ComputingMap garantit un seul thread ne tentera de remplir en cas d'absence et un autre thread demandant le même objet bloquera et attendra le résultat. Si vous supprimez une clé en cours de remplissage, ce thread et tous ceux qui sont actuellement en attente obtiendront toujours ce résultat, mais les requêtes suivantes relanceront la population.

Si vous avez besoin d'un internement, cette bibliothèque fournit l'excellente classe Interner avec une mise en cache fortement et faiblement référencée.

Answer 2

synchronisé prend jusqu'à 2 micro-secondes. Sauf si vous avez besoin de couper plus loin, vous pouvez être mieux avec la solution la plus simple.

BTW Vous pouvez écrire

SomeObject cached = cache.get(singletonInstance); 
if (cached == null) 
    cache.put(singletonInstance, cached = createSomeObject(singletonInstance)); 
return cached; 

Answer 3

Si « cache » est une carte (que je soupçonne qu'il est), ce problème est tout à fait différent qu'un simple problème de verrouillage revérifié.

Si le cache est un HashMap simple, le problème est en réalité bien pire; c'est-à-dire que votre "modèle à double vérification" proposé se comporte beaucoup moins bien qu'un simple double contrôle basé sur une référence. En fait, cela peut entraîner des exceptions ConcurrentModification, obtenir des valeurs incorrectes, ou même une boucle infinie.

Si elle est basée sur un HashMap simple, je suggère d'utiliser un ConcurrentHashMap comme première approche. Avec un ConcurrentHashMap, il n'y a pas de verrouillage explicite de votre part.

public SomeObject getObject(Identifier someIdentifier) { 
    // cache is a ConcurrentHashMap 

    // just check the cache, reference equality is not relevant just yet. 
    SomeObject cached = cache.get(someIdentifier); 
    if (cached != null) { 
     return cached; 
    }   

    Identifier singletonInstance = getUniqueIdentifier(someIdentifier); 
    SomeObject newInstance = createSomeObject(singletonInstance); 
    SombObject old = cache.putIfAbsent(singletonInstance, newInstance); 
    if (old != null) { 
     newInstance = old; 
    } 
    return newInstance; 
}