How to use lock on a Dictionary containing a list of objects in C#?

I have the following class:

public static class HotspotsCache
{
  private static Dictionary<short, List<HotSpot>> _companyHotspots = new Dictionary<int, List<HotSpot>>();
  private static object Lock = new object();


  public static List<HotSpot> GetCompanyHotspots(short companyId)
  {
     lock (Lock)
     {
       if (!_companyHotspots.ContainsKey(companyId))
       {
         RefreshCompanyHotspotCache(companyId);
       }

       return _companyHotspots[companyId];
     }
  }

  private static void RefreshCompanyHotspotCache(short companyId)
  {
    ....

    hotspots = ServiceProvider.Instance.GetService<HotspotsService>().GetHotSpots(..);
    _companyHotspots.Add(companyId, hotspots);

   ....
  }

The issue that I'm having is that the operation of getting the hotspots, in RefreshCompanyHotspotCache method, takes a lot of time . So while one thread is performing the cache refresh for a certain CompanyId, all the other threads are waiting until this operation is finished, although there could be threads that are req开发者_如何学JAVAuesting the list of hotspots for another companyId for which the list is already loaded in the dictionary. I would like these last threads not be locked. I also want that all threads that are requesting the list of hotspots for a company that is not yet loaded in the cache to wait until the list is fully retrieved and loaded in the dictionary.

Is there a way to lock only the threads that are reading/writing the cache for certain companyId (for which the refresh is taking place) and let the other threads that are requesting data for another company to do their job?

My thought was to use and array of locks

lock (companyLocks[companyId])
{
...
}

But that didn't solve anything. The threads dealing with one company are still waiting for threads that are refreshing the cache for other companies.

---

Use the Double-checked lock mechanism also mentioned by Snowbear - this will prevent your code locking when it doesn't actually need to.

With your idea of an individual lock per client, I've used this mechanism in the past, though I used a dictionary of locks. I made a utility class for getting a lock object from a key:

/// <summary>
/// Provides a mechanism to lock based on a data item being retrieved
/// </summary>
/// <typeparam name="T">Type of the data being used as a key</typeparam>
public class LockProvider<T> 
{
    private object _syncRoot = new object();
    private Dictionary<T, object> _lstLocks = new Dictionary<T, object>();

    /// <summary>
    /// Gets an object suitable for locking the specified data item
    /// </summary>
    /// <param name="key">The data key</param>
    /// <returns></returns>
    public object GetLock(T key)
    {
        if (!_lstLocks.ContainsKey(key))
        {
            lock (_syncRoot)
            {
                if (!_lstLocks.ContainsKey(key))
                    _lstLocks.Add(key, new object());
            }
        }
        return _lstLocks[key];
    }
}

So simply use this in the following manner...

private static LockProvider<short> _clientLocks = new LockProvider<short>();
private static Dictionary<short, List<HotSpot>> _companyHotspots = new Dictionary<short, List<HotSpot>>();

  public static List<HotSpot> GetCompanyHotspots(short companyId)
  {
      if (!_companyHotspots.ContainsKey(companyId)) 
      {
          lock (_clientLocks.GetLock(companyId)) 
          {
              if (!_companyHotspots.ContainsKey(companyId))
              {
                   // Add item to _companyHotspots here...
              }
      }
      return _companyHotspots[companyId];
  }

---

How about you only lock 1 thread, and let that update, while everyone else uses the old list?

private static Dictionary<short, List<HotSpot>> _companyHotspots = new Dictionary<short, List<HotSpot>>();
private static Dictionary<short, List<HotSpot>> _companyHotspotsOld = new Dictionary<short, List<HotSpot>>();
private static bool _hotspotsUpdating = false;
private static object Lock = new object();

public static List<HotSpot> GetCompanyHotspots(short companyId)
{
    if (!_hotspotsUpdating)
    {
        if (!_companyHotspots.ContainsKey(companyId))
        {
            lock (Lock)
            {
                _hotspotsUpdating = true;
                _companyHotspotsOld = _companyHotspots;
                RefreshCompanyHotspotCache(companyId);

                _hotspotsUpdating = false;
                return _companyHotspots[companyId];
            }
        }
        else
        {
            return _companyHotspots[companyId];
        }
    }
    else
    {
        return _companyHotspotsOld[companyId];
    }
}

---

Have you looked into ReaderWriterLockSlim? That should be able to let get finer grained locking where you only take a writelock when needed.

Another thing you may need to look out for is false sharing. I don't know how a lock is implemented exactly but if you lock on objects in an array they're bound to be close to each other in memory, possibly putting them on the same cacheline, so the lock may not behave as you expect.

Another idea, what happens if you change the last code snippet to

object l = companyLocks[companyId];
lock(l){

}

could be the lock statement wraps more here than intended.

GJ

---

New idea, with locking just the lists as they are created.

If you can guarantee that each company will have at least one hotspot, do this:

public static class HotspotsCache
{
    private static Dictionary<short, List<HotSpot>> _companyHotspots = new Dictionary<int, List<HotSpot>>();

    static HotspotsCache()
    {
        foreach(short companyId in allCompanies)
        {
            companyHotspots.Add(companyId, new List<HotSpot>());
        }
    }

    public static List<HotSpot> GetCompanyHotspots(short companyId)
    {
        List<HotSpots> result = _companyHotspots[companyId];

        if(result.Count == 0)
        {
            lock(result)
            {
                if(result.Count == 0)
                {
                    RefreshCompanyHotspotCache(companyId, result);
                }
            }
        }

        return result;
    }

    private static void RefreshCompanyHotspotCache(short companyId, List<HotSpot> resultList)
    {
        ....

        hotspots = ServiceProvider.Instance.GetService<HotspotsService>().GetHotSpots(..);
        resultList.AddRange(hotspots);

        ....
    }
}

Since the dictionary is being modified after its initial creation, no need to do any locking on it. We only need to lock the individual lists as we populate them, the read operation needs no locking (including the initial Count == 0).

---

If you're able to use .NET 4 then the answer is straightforward -- use a ConcurrentDictionary<K,V> instead and let that look after the concurrency details for you:

public static class HotSpotsCache
{
    private static readonly ConcurrentDictionary<short, List<HotSpot>>
        _hotSpotsMap = new ConcurrentDictionary<short, List<HotSpot>>();

    public static List<HotSpot> GetCompanyHotSpots(short companyId)
    {
        return _hotSpotsMap.GetOrAdd(companyId, id => LoadHotSpots(id));
    }

    private static List<HotSpot> LoadHotSpots(short companyId)
    {
        return ServiceProvider.Instance
                              .GetService<HotSpotsService>()
                              .GetHotSpots(/* ... */);
    }
}

If you're not able to use .NET 4 then your idea of using several more granular locks is a good one:

public static class HotSpotsCache
{
    private static readonly Dictionary<short, List<HotSpot>>
        _hotSpotsMap = new Dictionary<short, List<HotSpot>();

    private static readonly object _bigLock = new object();
    private static readonly Dictionary<short, object>
        _miniLocks = new Dictionary<short, object>();

    public static List<HotSpot> GetCompanyHotSpots(short companyId)
    {
        List<HotSpot> hotSpots;
        object miniLock;
        lock (_bigLock)
        {
            if (_hotSpotsMap.TryGetValue(companyId, out hotSpots))
                return hotSpots;

            if (!_miniLocks.TryGetValue(companyId, out miniLock))
            {
                miniLock = new object();
                _miniLocks.Add(companyId, miniLock);
            }
        }
        lock (miniLock)
        {
            if (!_hotSpotsMap.TryGetValue(companyId, out hotSpots))
            {
                hotSpots = LoadHotSpots(companyId);
                lock (_bigLock)
                {
                    _hotSpotsMap.Add(companyId, hotSpots);
                    _miniLocks.Remove(companyId);
                }
            }
            return hotSpots;
        }
    }

    private static List<HotSpot> LoadHotSpots(short companyId)
    {
        return ServiceProvider.Instance
                              .GetService<HotSpotsService>()
                              .GetHotSpots(/* ... */);
    }
}