Modèle de verrouillage pour une utilisation correcte de .NET MemoryCache

Question

Je suppose que ce code a des problèmes de simultanéité:

const string CacheKey = "CacheKey";
static string GetCachedData()
{
    string expensiveString =null;
    if (MemoryCache.Default.Contains(CacheKey))
    {
        expensiveString = MemoryCache.Default[CacheKey] as string;
    }
    else
    {
        CacheItemPolicy cip = new CacheItemPolicy()
        {
            AbsoluteExpiration = new DateTimeOffset(DateTime.Now.AddMinutes(20))
        };
        expensiveString = SomeHeavyAndExpensiveCalculation();
        MemoryCache.Default.Set(CacheKey, expensiveString, cip);
    }
    return expensiveString;
}

La raison pour laquelle le problème de concurrence, c'est que plusieurs threads peuvent obtenir une clé null, puis tentez d'insérer des données dans le cache.

Ce qui serait le plus court et le plus propre façon de faire ce code de simultanéité de la preuve? J'aime suivre un bon motif sur le cache de mon code associé. Un lien vers un article en ligne serait d'une grande aide.

Mise à JOUR:

Je suis venu avec ce code basé sur @Scott Chamberlain réponse. Peut-on trouver de rendement ou de problème de concurrence avec cela? Si cela fonctionne, cela permettrait d'économiser beaucoup de ligne de code et les erreurs.

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading.Tasks;
using System.Runtime.Caching;

namespace CachePoc
{
    class Program
    {
        static object everoneUseThisLockObject4CacheXYZ = new object();
        const string CacheXYZ = "CacheXYZ";
        static object everoneUseThisLockObject4CacheABC = new object();
        const string CacheABC = "CacheABC";

        static void Main(string[] args)
        {
            string xyzData = MemoryCacheHelper.GetCachedData<string>(CacheXYZ, everoneUseThisLockObject4CacheXYZ, 20, SomeHeavyAndExpensiveXYZCalculation);
            string abcData = MemoryCacheHelper.GetCachedData<string>(CacheABC, everoneUseThisLockObject4CacheXYZ, 20, SomeHeavyAndExpensiveXYZCalculation);
        }

        private static string SomeHeavyAndExpensiveXYZCalculation() {return "Expensive";}
        private static string SomeHeavyAndExpensiveABCCalculation() {return "Expensive";}

        public static class MemoryCacheHelper
        {
            public static T GetCachedData<T>(string cacheKey, object cacheLock, int cacheTimePolicyMinutes, Func<T> GetData)
                where T : class
            {
                //Returns null if the string does not exist, prevents a race condition where the cache invalidates between the contains check and the retreival.
                T cachedData = MemoryCache.Default.Get(cacheKey, null) as T;

                if (cachedData != null)
                {
                    return cachedData;
                }

                lock (cacheLock)
                {
                    //Check to see if anyone wrote to the cache while we where waiting our turn to write the new value.
                    cachedData = MemoryCache.Default.Get(cacheKey, null) as T;

                    if (cachedData != null)
                    {
                        return cachedData;
                    }

                    //The value still did not exist so we now write it in to the cache.
                    CacheItemPolicy cip = new CacheItemPolicy()
                    {
                        AbsoluteExpiration = new DateTimeOffset(DateTime.Now.AddMinutes(cacheTimePolicyMinutes))
                    };
                    cachedData = GetData();
                    MemoryCache.Default.Set(cacheKey, cachedData, cip);
                    return cachedData;
                }
            }
        }
    }
}

Answer 1

C'est mon 2ème itération du code. Parce qu' MemoryCache est thread-safe, vous n'avez pas besoin de vous concentrer sur la lecture initiale, vous pouvez simplement lire et si le cache renvoie la valeur null, alors ne la serrure de vérifier pour voir si vous avez besoin pour créer la chaîne. Il simplifie grandement le code.

const string CacheKey = "CacheKey";
static readonly object cacheLock = new object();
private static string GetCachedData()
{

    //Returns null if the string does not exist, prevents a race condition where the cache invalidates between the contains check and the retreival.
    var cachedString = MemoryCache.Default.Get(CacheKey, null) as string;

    if (cachedString != null)
    {
        return cachedString;
    }

    lock (cacheLock)
    {
        //Check to see if anyone wrote to the cache while we where waiting our turn to write the new value.
        cachedString = MemoryCache.Default.Get(CacheKey, null) as string;

        if (cachedString != null)
        {
            return cachedString;
        }

        //The value still did not exist so we now write it in to the cache.
        var expensiveString = SomeHeavyAndExpensiveCalculation();
        CacheItemPolicy cip = new CacheItemPolicy()
                              {
                                  AbsoluteExpiration = new DateTimeOffset(DateTime.Now.AddMinutes(20))
                              };
        MemoryCache.Default.Set(CacheKey, expensiveString, cip);
        return expensiveString;
    }
}

---

EDIT: Le code ci-dessous est inutile, mais je voulais le laisser montrer la méthode d'origine. Mon être utile aux futurs visiteurs qui sont à l'aide d'une collection différente qui est thread-safe lit mais non thread-safe écrit (presque toutes les classes de la System.Collections d'espace de noms est comme ça).

Voici comment j'allais le faire à l'aide de ReaderWriterLockSlim afin de protéger l'accès. Vous avez besoin de faire une sorte de "Double vérification de Verrouillage" pour voir si quelqu'un d'autre a créé l'élément mis en cache pendant que nous où l'attente pour prendre la serrure.

const string CacheKey = "CacheKey";
static readonly ReaderWriterLockSlim cacheLock = new ReaderWriterLockSlim();
static string GetCachedData()
{
    //First we do a read lock to see if it already exists, this allows multiple readers at the same time.
    cacheLock.EnterReadLock();
    try
    {
        //Returns null if the string does not exist, prevents a race condition where the cache invalidates between the contains check and the retreival.
        var cachedString = MemoryCache.Default.Get(CacheKey, null) as string;

        if (cachedString != null)
        {
            return cachedString;
        }
    }
    finally
    {
        cacheLock.ExitReadLock();
    }

    //Only one UpgradeableReadLock can exist at one time, but it can co-exist with many ReadLocks
    cacheLock.EnterUpgradeableReadLock();
    try
    {
        //We need to check again to see if the string was created while we where waiting to enter the EnterUpgradeableReadLock
        var cachedString = MemoryCache.Default.Get(CacheKey, null) as string;

        if (cachedString != null)
        {
            return cachedString;
        }

        //The entry still does not exist so we need to enter the write lock and create it, this will block till all the Readers flush.
        cacheLock.EnterWriteLock();
        try
        {
            CacheItemPolicy cip = new CacheItemPolicy()
            {
                AbsoluteExpiration = new DateTimeOffset(DateTime.Now.AddMinutes(20))
            };
            var expensiveString = SomeHeavyAndExpensiveCalculation();
            MemoryCache.Default.Set(CacheKey, expensiveString, cip);
            return expensiveString;
        }
        finally 
        {
            cacheLock.ExitWriteLock();
        }
    }
    finally
    {
        cacheLock.ExitUpgradeableReadLock();
    }
}

Answer 2

J'ai résolu ce problème en faisant usage de la AddOrGetExisting méthode sur la MemoryCache et l'utilisation de l'initialisation tardive.

Essentiellement, mon code ressemble à ceci:

static string GetCachedData(string key, DateTimeOffset offset)
{
    Lazy<String> lazyObject = new Lazy<String>(() => SomeHeavyAndExpensiveCalculationThatReturnsAString());
    var returnedLazyObject = MemoryCache.Default.AddOrGetExisting(key, lazyObject, offset);
    return ((Lazy<String>) returnedLazyObject).Value;
}

Le scénario du pire cas ici, c'est que vous créez le même Lazy objet deux fois. Mais c'est assez trivial. L'utilisation de l' AddOrGetExisting vous garantit que vous aurez jamais obtenir une instance de l' Lazy de l'objet, et donc, vous êtes également garanti qu'à appeler le coûteux méthode d'initialisation une fois.

Answer 3

Je suppose que ce code a des problèmes de simultanéité:

En fait, c'est très probablement bien, quoique avec une amélioration possible.

Maintenant, en général, le modèle où nous avons plusieurs threads paramètre une valeur partagée lors de la première utilisation, pour ne pas se verrouiller sur la valeur obtenue et peut être:

1. Désastreux - code ne supposent une seule instance existe.
2. Désastreux - le code qui obtient l'instance n'est pas ne peut tolérer un (ou peut-être un certain petit nombre) des opérations simultanées.
3. Désastreux - les moyens de stockage n'est pas thread-safe (par exemple, ont deux fils de l'ajout d'un dictionnaire et vous pouvez obtenir toutes sortes de vilaines erreurs).
4. Sous-optimale - la performance globale est pire que si le verrouillage a assuré qu'un thread a fait le travail de l'obtention de la valeur.
5. Optimale - le coût d'avoir plusieurs threads ne redondant de travail qui est moins que le coût de la prévention, d'autant que cela ne peut se faire au cours d'une période relativement brève.

Cependant, en considérant ici que MemoryCache peut expulser les entrées alors:

1. Si c'est désastreux pour avoir plus d'une instance puis en MemoryCache est la mauvaise approche.
2. Si vous devez empêcher la création simultanée, vous devez le faire au moment de la création.
3. MemoryCache est thread-safe en termes d'accès à l'objet, de sorte que n'est pas un problème ici.

Ces deux possibilités ont à être pensé, bien sûr, si la seule à avoir deux instances de la même chaîne existante peut être un problème si vous êtes en train de faire très particulier des optimisations qui ne s'appliquent pas ici*.

Donc, nous sommes de gauche, avec les possibilités:

1. Il est moins cher d'éviter le coût de dupliquer les appels d' SomeHeavyAndExpensiveCalculation().
2. Il est moins cher de ne pas éviter le coût de dupliquer les appels d' SomeHeavyAndExpensiveCalculation().

Et de travail qui peut être difficile (en effet, le genre de chose où il vaut la peine de profilage plutôt que de supposer que vous pouvez travailler sur). Il est utile de considérer ici que la plus évidente des moyens de verrouillage lors de l'insertion permettra d'éviter tous les ajouts de la cache, y compris ceux qui ne sont pas liés.

Cela signifie que si nous avions 50 threads en essayant de mettre 50 valeurs différentes, alors nous aurons à faire tous les 50 threads attendent les uns des autres, même si ils n'étaient même pas à faire le même calcul.

En tant que tel, vous êtes probablement mieux avec le code que vous avez, qu'avec un code qui évite la course-condition, et si la course-condition est un problème, vous l'avez très probablement besoin de gérer que quelque part d'autre, ou besoin d'une autre stratégie de mise en cache que celui qui expulse les anciennes entrées†.

La seule chose que je changerais c'est que je serais remplacer l'appel à Set() avec l'un d' AddOrGetExisting(). À partir de ce qui précède, il devrait être clair qu'il n'est probablement pas nécessaire, mais elle pourrait permettre l'obtention de nouveaux élément à percevoir, la réduction globale de l'utilisation de la mémoire et de permettre à un ratio plus élevé de la production à faible à élevé de génération de collections.

Donc oui, vous pouvez utiliser à double verrouillage pour empêcher la concurrence, mais soit la simultanéité n'est pas réellement un problème, ou votre stocker les valeurs dans le mauvais sens, ou double-verrouillage sur le magasin ne serait pas le meilleur moyen de les résoudre.

*Si vous ne connaissez qu'un chacun d'un ensemble de chaînes, vous pouvez optimiser la comparaison d'égalité, qui est le seul à avoir deux copies d'une chaîne de caractères peut être incorrecte plutôt que de simplement sous-optimale, mais vous voulez faire de très différents types de mise en cache pour que de faire sens. E. g. le tri XmlReader n'en interne.

†Très probablement l'un ou l'autre que les magasins indéfiniment, ou celui qui fait usage de références faibles donc il ne expulser les entrées si il n'y a pas d'utilisations existantes.

Answer 4

Un moyen rapide de faire une méthode thread-safe est d'utiliser la méthode Synchronisée concept:

[MethodImpl(MethodImplOptions.Synchronized)]
public void Foo () {}

Cela garantit qu'il n'existe qu'un seul thread à la fois qui exécute le code. Le problème avec cette méthode est cependant qu'une méthode peut être très long, et seule une petite partie est critique.

Une autre façon de le faire est de mettre en œuvre un lockfree/waitfree méthode. De telles méthodes de travail comme suit:

1. chaque discbased est annoté avec un timestamp
2. la méthode de calcule, en fonction de l'ancien discbased une nouvelle discbased.
3. avant de mettre le pointeur de la nouvelle structure, le fil vérifie si le timestamp a été modifié entre la lecture de la discbased et de contrôle.
4. si non, définir le pointeur, sinon, de les recalculer.