Question

J'ai une application Web sur laquelle je suis en train de faire des tests de charge / performance, en particulier sur une fonctionnalité dans laquelle plusieurs centaines d'utilisateurs devraient accéder à la même page et cliquer sur rafraîchir toutes les 10 secondes environ sur cette page. Nous avons constaté que l’une des améliorations que nous pouvions apporter à cette fonction consistait à mettre en cache les réponses du service Web pendant un certain temps, car les données ne changeaient pas.

Après avoir implémenté cette mise en cache de base, lors de tests supplémentaires, j’ai découvert que je n’avais pas pris en compte la manière dont les threads simultanés pouvaient accéder au cache en même temps. J'ai constaté qu'environ 100 ms, environ 50 threads essayaient d'extraire l'objet du cache, découvrant qu'il avait expiré, accédant au service Web pour extraire les données, puis replaçant l'objet dans le cache.

Le code d'origine ressemblait à quelque chose comme ceci:

private SomeData[] getSomeDataByEmail(WebServiceInterface service, String email) {

  final String key = "Data-" + email;
  SomeData[] data = (SomeData[]) StaticCache.get(key);

  if (data == null) {
      data = service.getSomeDataForEmail(email);

      StaticCache.set(key, data, CACHE_TIME);
  }
  else {
      logger.debug("getSomeDataForEmail: using cached object");
  }

  return data;
}

Ainsi, pour m'assurer qu'un seul thread appelait le service Web lorsque l'objet situé sur la clé expirait, je pensais que je devais synchroniser l'opération get / set du cache. La clé de cache serait un bon candidat pour la synchronisation d'un objet (de cette façon, les appels à cette méthode pour le courrier électronique b@b.com ne seraient pas bloqués par les appels de méthodes à a@a.com).

J'ai mis à jour la méthode pour qu'elle ressemble à ceci:

private SomeData[] getSomeDataByEmail(WebServiceInterface service, String email) {


  SomeData[] data = null;
  final String key = "Data-" + email;

  synchronized(key) {      
    data =(SomeData[]) StaticCache.get(key);

    if (data == null) {
        data = service.getSomeDataForEmail(email);
        StaticCache.set(key, data, CACHE_TIME);
    }
    else {
      logger.debug("getSomeDataForEmail: using cached object");
    }
  }

  return data;
}

J'ai également ajouté des lignes de journalisation pour des éléments tels que "avant le bloc de synchronisation", "dans le bloc de synchronisation", "sur le point de quitter le bloc de synchronisation" et "après le bloc de synchronisation", afin de pouvoir déterminer si j'étais effectivement synchroniser l'opération get / set.

Cependant, cela ne semble pas fonctionner. Mes journaux de test ont comme sortie:

  

(la sortie du journal est '' nom_fil '' '' nom du journal '' 'message')

  http-80-Processor253 jsp.view-page - getSomeDataForEmail: sur le point d'entrer dans un bloc de synchronisation
  http-80-Processor253 jsp.view-page - getSomeDataForEmail: à l'intérieur du bloc de synchronisation
  http-80-Processor253 cache.StaticCache - get: object at key [SomeData-test@test.com] a expiré

  http-80-Processor253 cache.StaticCache - get: key [SomeData-test@test.com] valeur retournée [null]
  http-80-Processor263 jsp.view-page - getSomeDataForEmail: sur le point d'entrer un bloc de synchronisation
  http-80-Processor263 jsp.view-page - getSomeDataForEmail: à l'intérieur du bloc de synchronisation
  http-80-Processor263 cache.StaticCache - get: object at key [SomeData-test@test.com] a expiré

  http-80-Processor263 cache.StaticCache - get: key [SomeData-test@test.com] renvoyant une valeur [null]
  http-80-Processor131 jsp.view-page - getSomeDataForEmail: sur le point d'entrer dans un bloc de synchronisation
  http-80-Processor131 jsp.view-page - getSomeDataForEmail: à l'intérieur du bloc de synchronisation
  http-80-Processor131 cache.StaticCache - get: object at key [SomeData-test@test.com] a expiré

  http-80-Processor131 cache.StaticCache - get: key [SomeData-test@test.com] renvoyant une valeur [null]
  http-80-Processor104 jsp.view-page - getSomeDataForEmail: à l'intérieur du bloc de synchronisation
  http-80-Processor104 cache.StaticCache - get: object at key [SomeData-test@test.com] a expiré

  http-80-Processor104 cache.StaticCache - get: key [SomeData-test@test.com] renvoyant une valeur [null]
  http-80-Processor252 jsp.view-page - getSomeDataForEmail: sur le point d'entrer dans un bloc de synchronisation
  http-80-Processor283 jsp.view-page - getSomeDataForEmail: sur le point d'entrer un bloc de synchronisation
  http-80-Processor2 jsp.view-page - getSomeDataForEmail: sur le point d'entrer dans le bloc de synchronisation
  http-80-Processor2 jsp.view-page - getSomeDataForEmail: dans le bloc de synchronisation

Je ne voulais voir qu'un seul thread à la fois entrer / sortir du bloc de synchronisation autour des opérations get / set.

Existe-t-il un problème de synchronisation sur les objets String? Je pensais que la clé de cache serait un bon choix car elle est unique pour l'opération, et même si la clé de chaîne finale est déclarée dans la méthode, je pensais que chaque thread obtiendrait une référence. le même objet et donc une synchronisation sur cet objet unique.

Qu'est-ce que je fais mal ici?

Mettre à jour : après un examen plus approfondi des journaux, il semble que les méthodes utilisent la même logique de synchronisation où la clé est toujours la même, telle que

.
final String key = "blah";
...
synchronized(key) { ...

ne présente pas le même problème de concurrence - un seul thread à la fois entre dans le bloc.

Mise à jour 2 : Merci à tous pour votre aide! J'ai accepté la première réponse à propos de intern () ing Strings, ce qui a résolu mon problème initial: plusieurs threads entraient dans des blocs synchronisés là où je pensais qu'ils ne devraient pas, car la touche a eu la même valeur.

Comme d'autres l'ont déjà fait remarquer, utiliser intern () à cette fin et effectuer une synchronisation sur ces chaînes s'avère en effet une mauvaise idée - lors de l'exécution de tests JMeter sur l'application Web pour simuler les résultats attendus. J'ai vu la taille du tas utilisé atteindre presque 1 Go en un peu moins de 20 minutes.

Actuellement, j’utilise la solution simple qui consiste à synchroniser l’ensemble de la méthode - mais j’aime vraiment les exemples de code fournis par martinprobst et MBCook, mais depuis que j'ai environ 7 getData ( ) méthodes actuellement dans cette classe (puisqu'il nécessite environ 7 éléments de données différents d'un service Web), je ne voulais pas ajouter de logique presque dupliquée pour obtenir et libérer des verrous pour chaque méthode. Mais c’est définitivement une information très précieuse pour un usage futur. Je pense que ce sont finalement les bonnes réponses sur la meilleure façon de réaliser une opération comme celle-ci en toute sécurité, et je donnerais plus de votes à ces réponses si je le pouvais!

Était-ce utile?

La solution

Sans que mon cerveau soit complètement engrené, à partir d'un balayage rapide de ce que vous dites, il semble que vous ayez besoin d'interner () vos chaînes:

final String firstkey = "Data-" + email;
final String key = firstkey.intern();

Deux chaînes avec la même valeur ne sont pas nécessairement le même objet.

Notez que cela peut introduire un nouveau point de conflit, car au plus profond de la VM, intern () peut avoir à acquérir un verrou. Je n'ai aucune idée de ce à quoi ressemblent les machines virtuelles modernes dans ce domaine, mais on espère qu'elles seront optimisées de façon diabolique.

Je suppose que vous savez que StaticCache doit toujours être thread-safe. Toutefois, le conflit devrait être minime comparé à ce que vous auriez si vous verrouillez le cache plutôt que la clé tout en appelant getSomeDataForEmail.

Réponse à la mise à jour de la question :

Je pense que c'est parce qu'un littéral de chaîne renvoie toujours le même objet. Dave Costa souligne dans un commentaire que c'est encore mieux que cela: un littéral donne toujours la représentation canonique. Ainsi, tous les littéraux de chaîne avec la même valeur n'importe où dans le programme produiraient le même objet.

Modifier

D'autres ont souligné que la synchronisation sur les chaînes internes est en réalité une très mauvaise idée - en partie parce que la création de chaînes internes est autorisée à les faire exister à perpétuité, et en partie parce que si plusieurs Le code, n'importe où dans votre programme, se synchronise sur les chaînes internes, vous avez des dépendances entre ces bits de code, et empêcher les blocages ou autres bogues peut être impossible.

Des stratégies pour éviter cela en stockant un objet verrou par chaîne de clé sont en cours de développement dans d'autres réponses en cours de frappe.

Voici une alternative - il utilise toujours un verrou singulier, mais nous savons que nous aurons besoin d’un de ceux-ci pour la mémoire cache de toute façon, et vous parliez de 50 threads, et non de 5000, afin que cela ne soit pas fatal. Je suppose également que le goulot d'étranglement des performances ici est le blocage lent des E / S dans DoSlowThing (), qui bénéficiera donc énormément de ne pas être sérialisé. Si ce n'est pas le goulot d'étranglement, alors:

  • Si le processeur est occupé, cette approche peut ne pas être suffisante et vous avez besoin d'une autre approche.
  • Si le processeur n'est pas occupé et que l'accès au serveur n'est pas un goulot d'étranglement, cette approche est excessive et vous pouvez aussi oublier le verrouillage à la fois par clé et placer un gros synchronisé (StaticCache) sur l'ensemble de l'opération. et faites-le facilement.

Évidemment, cette approche doit être testée pour sa scalabilité avant utilisation - Je ne garantis rien.

Ce code n'exige PAS que StaticCache soit synchronisé ou thread-safe. Cela doit être réexaminé si un autre code (par exemple le nettoyage programmé d'anciennes données) touche le cache.

IN_PROGRESS est une valeur fictive - pas tout à fait propre, mais le code est simple et vous évite d’avoir deux hashtables. Il ne gère pas InterruptedException car je ne sais pas ce que votre application veut faire dans ce cas. De même, si DoSlowThing () échoue systématiquement pour une clé donnée, le code actuel n’est pas vraiment élégant, car chaque thread le réessayera. Puisque je ne connais pas les critères d'échec et qu'ils sont susceptibles d'être temporaires ou permanents, je ne m'en occupe pas non plus, je m'assure simplement que les threads ne bloquent pas pour toujours. En pratique, vous pouvez placer une valeur de données dans le cache indiquant «non disponible», éventuellement avec une raison et un délai d'attente pour réessayer.

// do not attempt double-check locking here. I mean it.
synchronized(StaticObject) {
    data = StaticCache.get(key);
    while (data == IN_PROGRESS) {
        // another thread is getting the data
        StaticObject.wait();
        data = StaticCache.get(key);
    }
    if (data == null) {
        // we must get the data
        StaticCache.put(key, IN_PROGRESS, TIME_MAX_VALUE);
    }
}
if (data == null) {
    // we must get the data
    try {
        data = server.DoSlowThing(key);
    } finally {
        synchronized(StaticObject) {
            // WARNING: failure here is fatal, and must be allowed to terminate
            // the app or else waiters will be left forever. Choose a suitable
            // collection type in which replacing the value for a key is guaranteed.
            StaticCache.put(key, data, CURRENT_TIME);
            StaticObject.notifyAll();
        }
    }
}

Chaque fois que quelque chose est ajouté au cache, tous les threads se réveillent et vérifient le cache (quelle que soit la clé utilisée), ce qui permet d'optimiser les performances avec des algorithmes moins complexes. Cependant, une grande partie de ce travail sera effectué pendant les longs blocages de mémoire sur le processeur inactif, il se peut donc que ce ne soit pas un problème.

Ce code peut être associé à plusieurs caches si vous définissez des abstractions appropriées pour le cache et son verrou associé, les données qu’il renvoie, le mannequin IN_PROGRESS et l’opération lente à effectuer. Faire rouler le tout dans une méthode sur le cache peut ne pas être une mauvaise idée.

Autres conseils

La synchronisation sur une chaîne intern'd n’est peut-être pas une bonne idée. En l’internant, la chaîne se transforme en objet global. Si vous synchronisez sur les mêmes chaînes internées dans différentes parties de votre application, vous risquez d’obtenir des problèmes de synchronisation vraiment étranges et fondamentalement impossibles à résoudre, tels que des impasses. Cela peut sembler improbable, mais quand cela arrive, vous êtes vraiment foutu. En règle générale, synchronisez uniquement sur un objet local pour lequel vous êtes absolument certain qu'aucun code en dehors de votre module ne pourra le verrouiller.

Dans votre cas, vous pouvez utiliser une hashtable synchronisée pour stocker des objets verrouillables pour vos clés.

Exemple:

Object data = StaticCache.get(key, ...);
if (data == null) {
  Object lock = lockTable.get(key);
  if (lock == null) {
    // we're the only one looking for this
    lock = new Object();
    synchronized(lock) {
      lockTable.put(key, lock);
      // get stuff
      lockTable.remove(key);
    }
  } else {
    synchronized(lock) {
      // just to wait for the updater
    }
    data = StaticCache.get(key);
  }
} else {
  // use from cache
}

Ce code a une condition de concurrence critique, dans laquelle deux threads peuvent placer un objet l'un après l'autre dans la table de verrouillage. Cela ne devrait toutefois pas poser de problème, car vous ne disposez alors que d’un seul thread supplémentaire appelant le service Web et mettant à jour le cache, ce qui ne devrait pas poser de problème.

Si vous invalidez le cache après un certain temps, vous devez vérifier si les données sont à nouveau nulles après les avoir extraites du cache, dans le cas lock! = null.

Sinon, et beaucoup plus facilement, vous pouvez synchroniser la méthode de recherche du cache ("getSomeDataByEmail"). Cela signifie que tous les threads doivent se synchroniser lorsqu'ils accèdent au cache, ce qui peut être un problème de performances. Mais comme toujours, essayez d'abord cette solution simple et voyez si c'est vraiment un problème! Dans de nombreux cas, cela ne devrait pas être le cas, car vous passez probablement beaucoup plus de temps à traiter le résultat qu'à la synchronisation.

Les chaînes ne sont pas pour la synchronisation. Si vous devez synchroniser avec un ID de chaîne, vous pouvez utiliser cette chaîne pour créer un mutex (voir " synchronisation sur un identifiant "). Que le coût de cet algorithme en vaille la peine dépend de si l'appel de votre service implique une entrée / sortie importante.

Aussi:

  • J'espère que les méthodes StaticCache.get () et set () sont threadsafe.
  • String.intern () a un coût (qui varie selon les implémentations de machine virtuelle) et doit être utilisé avec précaution.

D'autres ont suggéré d'interner les chaînes et cela fonctionnera.

Le problème est que Java doit conserver les chaînes internes. On m'a dit que c'est le cas même si vous ne tenez pas de référence car la valeur doit être la même à la prochaine utilisation de cette chaîne. Cela signifie que l’internalisation de toutes les chaînes peut commencer à saturer la mémoire, ce qui, avec la charge que vous décrivez, pourrait poser un gros problème.

J'ai vu deux solutions à cela:

Vous pouvez synchroniser sur un autre objet

À la place de l'email, créez un objet contenant l'email (par exemple, l'objet User) contenant la valeur de l'email en tant que variable. Si vous avez déjà un autre objet représentant la personne (disons que vous avez déjà extrait quelque chose de la base de données en fonction de son courrier électronique), vous pouvez l'utiliser. En implémentant la méthode equals et la méthode hashcode, vous pouvez vous assurer que Java considère les objets de la même manière lorsque vous effectuez un static.catains () cache pour savoir si les données sont déjà dans le cache (vous devrez synchroniser le cache). ).

En fait, vous pouvez conserver une deuxième carte pour que les objets se verrouillent. Quelque chose comme ça:

Map<String, Object> emailLocks = new HashMap<String, Object>();

Object lock = null;

synchronized (emailLocks) {
    lock = emailLocks.get(emailAddress);

    if (lock == null) {
        lock = new Object();
        emailLocks.put(emailAddress, lock);
    }
}

synchronized (lock) {
    // See if this email is in the cache
    // If so, serve that
    // If not, generate the data

    // Since each of this person's threads synchronizes on this, they won't run
    // over eachother. Since this lock is only for this person, it won't effect
    // other people. The other synchronized block (on emailLocks) is small enough
    // it shouldn't cause a performance problem.
}

Ceci empêchera 15 extractions sur la même adresse électronique à la fois. Vous aurez besoin de quelque chose pour empêcher trop d'entrées de se retrouver dans la carte emailLocks. L'utilisation de LRUMap de Apache Commons serait fais-le.

Cela nécessitera quelques modifications, mais cela pourrait résoudre votre problème.

Utiliser une clé différente

Si vous êtes prêt à supporter d'éventuelles erreurs (j'ignore à quel point c'est important), vous pouvez utiliser le hashcode de la chaîne comme clé. Les internautes n'ont pas besoin d'être internés.

Résumé

J'espère que cela aide. Le filetage est amusant, n'est-ce pas? Vous pouvez également utiliser la session pour définir une valeur qui signifie "je travaille déjà sur la recherche de cette". et vérifiez que pour voir si le deuxième (le troisième, nième) thread doit essayer de créer le ou attendez simplement que le résultat apparaisse dans le cache. Je suppose que j'avais trois suggestions.

Vous pouvez utiliser les utilitaires de concurrence 1.5 pour créer un cache conçu pour permettre plusieurs accès simultanés et un seul point d’addition (c’est-à-dire un seul thread exécutant l’objet coûteux "création"):

 private ConcurrentMap<String, Future<SomeData[]> cache;
 private SomeData[] getSomeDataByEmail(final WebServiceInterface service, final String email) throws Exception {

  final String key = "Data-" + email;
  Callable<SomeData[]> call = new Callable<SomeData[]>() {
      public SomeData[] call() {
          return service.getSomeDataForEmail(email);
      }
  }
  FutureTask<SomeData[]> ft; ;
  Future<SomeData[]> f = cache.putIfAbsent(key, ft= new FutureTask<SomeData[]>(call)); //atomic
  if (f == null) { //this means that the cache had no mapping for the key
      f = ft;
      ft.run();
  }
  return f.get(); //wait on the result being available if it is being calculated in another thread
}

Évidemment, cela ne gère pas les exceptions comme vous le souhaitez et le cache ne contient pas d'expulsion. Vous pouvez peut-être l'utiliser comme base pour modifier votre classe StaticCache, cependant.

Utilisez un cadre de mise en cache décent tel que ehcache .

Implémenter un bon cache n’est pas aussi facile que certains le pensent.

En ce qui concerne le commentaire selon lequel String.intern () est une source de fuites de mémoire, ce n’est pas le cas. Les chaînes internées sont collectées, cela risque de prendre un peu plus de temps car, sur certaines machines virtuelles (SUN), elles sont stockées dans l’espace Perm qui est uniquement affecté par les GC complètes.

Voici une solution Java 8 courte et sûre qui utilise une mappe d'objets de verrouillage dédiés pour la synchronisation:

private static final Map<String, Object> keyLocks = new ConcurrentHashMap<>();

private SomeData[] getSomeDataByEmail(WebServiceInterface service, String email) {
    final String key = "Data-" + email;
    synchronized (keyLocks.computeIfAbsent(key, k -> new Object())) {
        SomeData[] data = StaticCache.get(key);
        if (data == null) {
            data = service.getSomeDataForEmail(email);
            StaticCache.set(key, data);
        }
    }
    return data;
}

L’inconvénient est que les clés et les objets verrouillés resteraient indéfiniment dans la carte.

Ceci peut être contourné comme ceci:

private SomeData[] getSomeDataByEmail(WebServiceInterface service, String email) {
    final String key = "Data-" + email;
    synchronized (keyLocks.computeIfAbsent(key, k -> new Object())) {
        try {
            SomeData[] data = StaticCache.get(key);
            if (data == null) {
                data = service.getSomeDataForEmail(email);
                StaticCache.set(key, data);
            }
        } finally {
            keyLocks.remove(key); // vulnerable to race-conditions
        }
    }
    return data;
}

Mais alors les clés populaires seraient constamment réinsérées dans la carte, les objets de verrouillage étant réalloués.

Mise à jour : cela laisse une possibilité de concurrence lorsque deux threads entrent simultanément dans une section synchronisée pour la même clé mais avec des verrous différents.

Il est donc peut-être plus sûr et plus efficace d'utiliser d'exclure le cache de la goyave :

private static final LoadingCache<String, Object> keyLocks = CacheBuilder.newBuilder()
        .expireAfterAccess(10, TimeUnit.MINUTES) // max lock time ever expected
        .build(CacheLoader.from(Object::new));

private SomeData[] getSomeDataByEmail(WebServiceInterface service, String email) {
    final String key = "Data-" + email;
    synchronized (keyLocks.getUnchecked(key)) {
        SomeData[] data = StaticCache.get(key);
        if (data == null) {
            data = service.getSomeDataForEmail(email);
            StaticCache.set(key, data);
        }
    }
    return data;
}

Notez qu'il est supposé ici que StaticCache est thread-safe et ne souffrirait pas de lectures et écritures simultanées pour différentes clés.

Votre problème principal n’est pas simplement qu’il peut y avoir plusieurs instances de String avec la même valeur. Le problème principal est qu'il ne faut qu'un seul moniteur sur lequel se synchroniser pour accéder à l'objet StaticCache. Sinon, plusieurs threads pourraient modifier simultanément StaticCache (bien que sous des clés différentes), ce qui ne prend probablement pas en charge la modification simultanée.

L'appel:

   final String key = "Data-" + email;

crée un nouvel objet chaque fois que la méthode est appelée. Parce que cet objet est ce que vous utilisez pour verrouiller et que chaque appel à cette méthode crée un nouvel objet, vous ne synchronisez pas vraiment l'accès à la carte en fonction de la clé.

Ceci explique plus en détail votre modification. Lorsque vous avez une chaîne statique, cela fonctionnera.

L'utilisation de intern () résout le problème, car elle renvoie la chaîne d'un pool interne conservé par la classe String, ce qui garantit que si deux chaînes sont égales, celle du pool sera utilisée. Voir

http: / /java.sun.com/j2se/1.4.2/docs/api/java/lang/String.html#intern ()

Cette question me semble un peu trop large et elle a donc suscité un ensemble de réponses tout aussi large. Je vais donc essayer de répondre à la question J'ai été redirigé depuis, malheureusement celui-ci a été fermé en double.

public class ValueLock<T> {

    private Lock lock = new ReentrantLock();
    private Map<T, Condition> conditions  = new HashMap<T, Condition>();

    public void lock(T t){
        lock.lock();
        try {
            while (conditions.containsKey(t)){
                conditions.get(t).awaitUninterruptibly();
            }
            conditions.put(t, lock.newCondition());
        } finally {
            lock.unlock();
        }
    }

    public void unlock(T t){
        lock.lock();
        try {
            Condition condition = conditions.get(t);
            if (condition == null)
                throw new IllegalStateException();// possibly an attempt to release what wasn't acquired
            conditions.remove(t);
            condition.signalAll();
        } finally {
            lock.unlock();
        }
    }

Lors de l'opération (extérieure) verrou , le verrou (intérieur) est acquis pour obtenir un accès exclusif à la carte pendant un court instant. Si l'objet correspondant est déjà dans la carte, le le fil attendra, sinon, la nouvelle Condition sera ajoutée à la carte, libérez le verrou (interne) et continuez, et le verrou (extérieur) est considéré comme obtenu. L’opération unlock , qui commence par acquérir un verrou (interne), signalera la Condition , puis supprimera l’objet de la carte.

La classe n'utilise pas la version simultanée de Map , car chaque accès à celle-ci est protégé par un verrou unique (interne).

Notez que la sémantique de la méthode lock () de cette classe est différente de celle de ReentrantLock.lock () , la répétition lock () les appels sans unlock () couplé suspendent indéfiniment le thread en cours.

Un exemple d'utilisation qui pourrait être applicable à la situation, décrit le PO

    ValueLock<String> lock = new ValueLock<String>();
    // ... share the lock   
    String email = "...";
    try {
        lock.lock(email);
        //... 
    } finally {
        lock.unlock(email);
    }

C'est un peu tard, mais il y a pas mal de code incorrect présenté ici.

Dans cet exemple:

private SomeData[] getSomeDataByEmail(WebServiceInterface service, String email) {


  SomeData[] data = null;
  final String key = "Data-" + email;

  synchronized(key) {      
    data =(SomeData[]) StaticCache.get(key);

    if (data == null) {
        data = service.getSomeDataForEmail(email);
        StaticCache.set(key, data, CACHE_TIME);
    }
    else {
      logger.debug("getSomeDataForEmail: using cached object");
    }
  }

  return data;
}

La synchronisation est mal dimensionnée. Pour un cache statique prenant en charge une API get / put, il doit exister au moins une synchronisation autour des opérations de type get et getIfAbsentPut, pour un accès sécurisé au cache. L'étendue de la synchronisation sera le cache lui-même.

Si des mises à jour doivent être apportées aux éléments de données eux-mêmes, cela ajoute une couche supplémentaire de synchronisation, qui devrait figurer sur les éléments de données individuels.

SynchronizedMap peut être utilisé à la place de la synchronisation explicite, mais il convient de rester prudent. Si des API incorrectes sont utilisées (get et put à la place de putIfAbsent), les opérations ne disposeront pas de la synchronisation nécessaire, malgré l'utilisation de la mappe synchronisée. Notez les complications introduites par l’utilisation de putIfAbsent: Soit la valeur de vente doit être calculée même dans les cas où elle n’est pas nécessaire (car elle ne peut pas savoir si la valeur de vente est nécessaire avant que le contenu du cache ne soit examiné), ou nécessite une analyse minutieuse. utilisation de la délégation (par exemple en utilisant Future, ce qui fonctionne, mais constitue en quelque sorte un déséquilibre; voir ci-dessous), où la valeur de vente est obtenue à la demande si nécessaire.

L’utilisation des contrats à terme est possible, mais semble plutôt délicate et peut-être un peu trop d’ingénierie. L'API future est au cœur des opérations asynchrones, en particulier des opérations qui risquent de ne pas s'achever immédiatement. Impliquer le futur ajoute très probablement une couche de création de threads - des complications supplémentaires probablement inutiles.

Le principal problème lié à l'utilisation de Future pour ce type d'opération est que Future est intrinsèquement lié au multi-threading. Utiliser Future quand un nouveau thread n'est pas nécessaire, c'est ignorer une grande partie de la machinerie de Future, ce qui en fait une API trop lourde pour cet usage.

Pourquoi ne pas simplement restituer une page HTML statique qui est servie à l'utilisateur et régénérée toutes les x minutes?

Je suggérerais également de supprimer complètement la concaténation de chaînes si vous n'en avez pas besoin.

final String key = "Data-" + email;

Le cache contient-il d'autres objets ou types d'objets utilisant l'adresse électronique dont vous avez besoin? "Données". au début de la clé?

sinon, je ferais juste que

final String key = email;

et vous évitez également toute cette création de chaîne supplémentaire.

synchronisation d'une autre manière sur l'objet chaîne:

String cacheKey = ...;

    Object obj = cache.get(cacheKey)

    if(obj==null){
    synchronized (Integer.valueOf(Math.abs(cacheKey.hashCode()) % 127)){
          obj = cache.get(cacheKey)
         if(obj==null){
             //some cal obtain obj value,and put into cache
        }
    }
}

Si d'autres personnes rencontrent un problème similaire, le code suivant fonctionne, pour autant que je sache:

import java.util.Map;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.atomic.AtomicInteger;
import java.util.function.Supplier;

public class KeySynchronizer<T> {

    private Map<T, CounterLock> locks = new ConcurrentHashMap<>();

    public <U> U synchronize(T key, Supplier<U> supplier) {
        CounterLock lock = locks.compute(key, (k, v) -> 
                v == null ? new CounterLock() : v.increment());
        synchronized (lock) {
            try {
                return supplier.get();
            } finally {
                if (lock.decrement() == 0) {
                    // Only removes if key still points to the same value,
                    // to avoid issue described below.
                    locks.remove(key, lock);
                }
            }
        }
    }

    private static final class CounterLock {

        private AtomicInteger remaining = new AtomicInteger(1);

        private CounterLock increment() {
            // Returning a new CounterLock object if remaining = 0 to ensure that
            // the lock is not removed in step 5 of the following execution sequence:
            // 1) Thread 1 obtains a new CounterLock object from locks.compute (after evaluating "v == null" to true)
            // 2) Thread 2 evaluates "v == null" to false in locks.compute
            // 3) Thread 1 calls lock.decrement() which sets remaining = 0
            // 4) Thread 2 calls v.increment() in locks.compute
            // 5) Thread 1 calls locks.remove(key, lock)
            return remaining.getAndIncrement() == 0 ? new CounterLock() : this;
        }

        private int decrement() {
            return remaining.decrementAndGet();
        }
    }
}

Dans le cas du PO, il serait utilisé comme ceci:

private KeySynchronizer<String> keySynchronizer = new KeySynchronizer<>();

private SomeData[] getSomeDataByEmail(WebServiceInterface service, String email) {
    String key = "Data-" + email;
    return keySynchronizer.synchronize(key, () -> {
        SomeData[] existing = (SomeData[]) StaticCache.get(key);
        if (existing == null) {
            SomeData[] data = service.getSomeDataForEmail(email);
            StaticCache.set(key, data, CACHE_TIME);
            return data;
        }
        logger.debug("getSomeDataForEmail: using cached object");
        return existing;
    });
}

Si rien ne doit être renvoyé du code synchronisé, la méthode synchronize peut être écrite comme suit:

public void synchronize(T key, Runnable runnable) {
    CounterLock lock = locks.compute(key, (k, v) -> 
            v == null ? new CounterLock() : v.increment());
    synchronized (lock) {
        try {
            runnable.run();
        } finally {
            if (lock.decrement() == 0) {
                // Only removes if key still points to the same value,
                // to avoid issue described below.
                locks.remove(key, lock);
            }
        }
    }
}

J'ai ajouté une petite classe de verrouillage pouvant verrouiller / synchroniser toutes les clés, y compris les chaînes.

Voir la mise en oeuvre pour Java 8, Java 6 et un petit test.

Java 8:

public class DynamicKeyLock<T> implements Lock
{
    private final static ConcurrentHashMap<Object, LockAndCounter> locksMap = new ConcurrentHashMap<>();

    private final T key;

    public DynamicKeyLock(T lockKey)
    {
        this.key = lockKey;
    }

    private static class LockAndCounter
    {
        private final Lock lock = new ReentrantLock();
        private final AtomicInteger counter = new AtomicInteger(0);
    }

    private LockAndCounter getLock()
    {
        return locksMap.compute(key, (key, lockAndCounterInner) ->
        {
            if (lockAndCounterInner == null) {
                lockAndCounterInner = new LockAndCounter();
            }
            lockAndCounterInner.counter.incrementAndGet();
            return lockAndCounterInner;
        });
    }

    private void cleanupLock(LockAndCounter lockAndCounterOuter)
    {
        if (lockAndCounterOuter.counter.decrementAndGet() == 0)
        {
            locksMap.compute(key, (key, lockAndCounterInner) ->
            {
                if (lockAndCounterInner == null || lockAndCounterInner.counter.get() == 0) {
                    return null;
                }
                return lockAndCounterInner;
            });
        }
    }

    @Override
    public void lock()
    {
        LockAndCounter lockAndCounter = getLock();

        lockAndCounter.lock.lock();
    }

    @Override
    public void unlock()
    {
        LockAndCounter lockAndCounter = locksMap.get(key);
        lockAndCounter.lock.unlock();

        cleanupLock(lockAndCounter);
    }


    @Override
    public void lockInterruptibly() throws InterruptedException
    {
        LockAndCounter lockAndCounter = getLock();

        try
        {
            lockAndCounter.lock.lockInterruptibly();
        }
        catch (InterruptedException e)
        {
            cleanupLock(lockAndCounter);
            throw e;
        }
    }

    @Override
    public boolean tryLock()
    {
        LockAndCounter lockAndCounter = getLock();

        boolean acquired = lockAndCounter.lock.tryLock();

        if (!acquired)
        {
            cleanupLock(lockAndCounter);
        }

        return acquired;
    }

    @Override
    public boolean tryLock(long time, TimeUnit unit) throws InterruptedException
    {
        LockAndCounter lockAndCounter = getLock();

        boolean acquired;
        try
        {
            acquired = lockAndCounter.lock.tryLock(time, unit);
        }
        catch (InterruptedException e)
        {
            cleanupLock(lockAndCounter);
            throw e;
        }

        if (!acquired)
        {
            cleanupLock(lockAndCounter);
        }

        return acquired;
    }

    @Override
    public Condition newCondition()
    {
        LockAndCounter lockAndCounter = locksMap.get(key);

        return lockAndCounter.lock.newCondition();
    }
}

Java 6:

la classe publique DynamicKeyLock implémente Lock     {         finale finale privée privée ConcurrentHashMap locksMap = new ConcurrentHashMap ();         touche T finale privée;

    public DynamicKeyLock(T lockKey) {
        this.key = lockKey;
    }

    private static class LockAndCounter {
        private final Lock lock = new ReentrantLock();
        private final AtomicInteger counter = new AtomicInteger(0);
    }

    private LockAndCounter getLock()
    {
        while (true) // Try to init lock
        {
            LockAndCounter lockAndCounter = locksMap.get(key);

            if (lockAndCounter == null)
            {
                LockAndCounter newLock = new LockAndCounter();
                lockAndCounter = locksMap.putIfAbsent(key, newLock);

                if (lockAndCounter == null)
                {
                    lockAndCounter = newLock;
                }
            }

            lockAndCounter.counter.incrementAndGet();

            synchronized (lockAndCounter)
            {
                LockAndCounter lastLockAndCounter = locksMap.get(key);
                if (lockAndCounter == lastLockAndCounter)
                {
                    return lockAndCounter;
                }
                // else some other thread beat us to it, thus try again.
            }
        }
    }

    private void cleanupLock(LockAndCounter lockAndCounter)
    {
        if (lockAndCounter.counter.decrementAndGet() == 0)
        {
            synchronized (lockAndCounter)
            {
                if (lockAndCounter.counter.get() == 0)
                {
                    locksMap.remove(key);
                }
            }
        }
    }

    @Override
    public void lock()
    {
        LockAndCounter lockAndCounter = getLock();

        lockAndCounter.lock.lock();
    }

    @Override
    public void unlock()
    {
        LockAndCounter lockAndCounter = locksMap.get(key);
        lockAndCounter.lock.unlock();

        cleanupLock(lockAndCounter);
    }


    @Override
    public void lockInterruptibly() throws InterruptedException
    {
        LockAndCounter lockAndCounter = getLock();

        try
        {
            lockAndCounter.lock.lockInterruptibly();
        }
        catch (InterruptedException e)
        {
            cleanupLock(lockAndCounter);
            throw e;
        }
    }

    @Override
    public boolean tryLock()
    {
        LockAndCounter lockAndCounter = getLock();

        boolean acquired = lockAndCounter.lock.tryLock();

        if (!acquired)
        {
            cleanupLock(lockAndCounter);
        }

        return acquired;
    }

    @Override
    public boolean tryLock(long time, TimeUnit unit) throws InterruptedException
    {
        LockAndCounter lockAndCounter = getLock();

        boolean acquired;
        try
        {
            acquired = lockAndCounter.lock.tryLock(time, unit);
        }
        catch (InterruptedException e)
        {
            cleanupLock(lockAndCounter);
            throw e;
        }

        if (!acquired)
        {
            cleanupLock(lockAndCounter);
        }

        return acquired;
    }

    @Override
    public Condition newCondition()
    {
        LockAndCounter lockAndCounter = locksMap.get(key);

        return lockAndCounter.lock.newCondition();
    }
}

Test:

public class DynamicKeyLockTest
{
    @Test
    public void testDifferentKeysDontLock() throws InterruptedException
    {
        DynamicKeyLock<Object> lock = new DynamicKeyLock<>(new Object());
        lock.lock();
        AtomicBoolean anotherThreadWasExecuted = new AtomicBoolean(false);
        try
        {
            new Thread(() ->
            {
                DynamicKeyLock<Object> anotherLock = new DynamicKeyLock<>(new Object());
                anotherLock.lock();
                try
                {
                    anotherThreadWasExecuted.set(true);
                }
                finally
                {
                    anotherLock.unlock();
                }
            }).start();
            Thread.sleep(100);
        }
        finally
        {
            Assert.assertTrue(anotherThreadWasExecuted.get());
            lock.unlock();
        }
    }

    @Test
    public void testSameKeysLock() throws InterruptedException
    {
        Object key = new Object();
        DynamicKeyLock<Object> lock = new DynamicKeyLock<>(key);
        lock.lock();
        AtomicBoolean anotherThreadWasExecuted = new AtomicBoolean(false);
        try
        {
            new Thread(() ->
            {
                DynamicKeyLock<Object> anotherLock = new DynamicKeyLock<>(key);
                anotherLock.lock();
                try
                {
                    anotherThreadWasExecuted.set(true);
                }
                finally
                {
                    anotherLock.unlock();
                }
            }).start();
            Thread.sleep(100);
        }
        finally
        {
            Assert.assertFalse(anotherThreadWasExecuted.get());
            lock.unlock();
        }
    }
}

Dans votre cas, vous pouvez utiliser quelque chose comme ceci (cela ne perd pas de mémoire):

private Synchronizer<String> synchronizer = new Synchronizer();

private SomeData[] getSomeDataByEmail(WebServiceInterface service, String email) {
    String key = "Data-" + email;

    return synchronizer.synchronizeOn(key, () -> {

        SomeData[] data = (SomeData[]) StaticCache.get(key);
        if (data == null) {
            data = service.getSomeDataForEmail(email);
            StaticCache.set(key, data, CACHE_TIME);
        } else {
          logger.debug("getSomeDataForEmail: using cached object");
        }
        return data;

    });
}

pour l'utiliser, il vous suffit d'ajouter une dépendance:

compile 'com.github.matejtymes:javafixes:1.3.0'

Vous pouvez utiliser String.intern en toute sécurité pour la synchronisation si vous pouvez raisonnablement garantir que la valeur de la chaîne est unique sur votre système. Les UUIDS sont un bon moyen d’aborder cela. Vous pouvez associer un UUID à votre clé de chaîne réelle, via un cache, une carte ou même stocker l’uuid en tant que champ sur votre objet entité.

    @Service   
    public class MySyncService{

      public Map<String, String> lockMap=new HashMap<String, String>();

      public void syncMethod(String email) {

        String lock = lockMap.get(email);
        if(lock==null) {
            lock = UUID.randomUUID().toString();
            lockMap.put(email, lock);
        }   

        synchronized(lock.intern()) {
                //do your sync code here
        }
    }
Licencié sous: CC-BY-SA avec attribution
Non affilié à StackOverflow
scroll top