2015-03-31 78 views
1

我有一個關鍵的任務映射,我需要運行的任務只有當給定的任務尚未運行。僞代碼如下。我相信有很大的改進空間。我鎖定在地圖上,因此幾乎可以序列化訪問CacheFreshener。有沒有更好的方法來做到這一點?我們知道,當我試圖鎖定密鑰k1時,高速緩存清新器調用密鑰k2等待鎖定沒有任何意義。鎖定和刷新實施

class CacheFreshener 
{ 
    private ConcurrentDictionary<string,bool> lockMap; 

    public RefreshData(string key, Func<string, bool> cacheMissAction) 
    { 
     lock(lockMap) 
     { 
      if (lockMap.ContainsKey(key)) 
      { 
       // no-op 
       return; 
      } 
      else 
      { 
       lockMap.Add(key, true); 
      } 
     } 

     // if you are here means task is not already present 
     cacheMissAction(key); 

     lock(lockMap) // Do we need to lock here?? 
     { 
      lockMap.Remove(key); 
     } 
    } 

} 
+1

問題的類型'有沒有更好的方法?'通常去CodeReview SE。 – SimpleVar 2015-03-31 01:43:27

+0

@YoryeNathan也許你是對的,然而**代碼**是**總是**關閉[codereview.se]的話題。 – nhgrif 2015-03-31 01:45:41

+0

@nhgrif雖然他稱之爲僞代碼,但它僅僅是C#。我相信他正在考慮更多地朝着「未完成的代碼」或「正在進行的工作」的方向發展 - 這不是僞語言中的代碼。無論哪種方式,它只是C#。 – SimpleVar 2015-03-31 01:46:44

回答

0

根據要求,這裏是一個詳細的說明我得到什麼相對於我的評論&hellip;

這裏的基本問題似乎是併發問題,即一次訪問同一對象的兩個或多個線程。這是ConcurrentDictionary專爲此設計的場景。如果分別使用ContainsKey()Add()IDictionary方法,則需要明確的同步(但僅限於該操作;在此特定情況下,在調用Remove()時不會嚴格需要)以確保這些操作作爲單個原子操作來執行。但類會預期這種需求,並且包括用於完成相同操作的TryAdd()方法,而不需要顯式同步。

<給出一旁>
這是不完全清楚,我的代碼示例背後的意圖。該代碼似乎僅用於在調用cacheMissAction委託期間將對象存儲在「緩存」中。密鑰在之後立即被刪除。所以它看起來似乎並沒有真正緩存任何東西。它只是阻止多個線程同時調用cacheMissAction(隨後的線程將無法調用它,但也不能指望它在調用RefreshData()方法時已完成)。
< /拋開>

但走的是代碼示例給出,很明顯,沒有明確的鎖定實際需要。 ConcurrentDictionary類已經提供了線程安全訪問(即,當從多個線程同時使用數據結構時不會發生錯誤),並且它提供了方法作爲添加密鑰(及其值的機制,儘管這裏總是一個bool字面值爲true),以確保一次只有一個線程在字典中有一個鍵。

所以我們可以重寫代碼看起來像這樣來代替,而完成同樣的目標:

private ConcurrentDictionary<string,bool> lockMap; 

public RefreshData(string key, Func<string, bool> cacheMissAction) 
{ 
    if (!lockMap.TryAdd(key, true)) 
    { 
     return; 
    } 

    // if you are here means task was not already present 
    cacheMissAction(key); 

    lockMap.Remove(key); 
} 

是需要或者添加或刪除無lock聲明,作爲TryAdd()處理整個「檢查重點並添加如果不存在「操作原子。


我會注意到,使用字典來完成集合的工作可能被認爲是低效的。如果這個集合可能不是很大,那麼這沒什麼大不了的,但是我確實發現微軟選擇犯了同樣的錯誤是因爲他們在前仿製時期必須使用非通用字典對象HashtableHashSet<T>出現之前存儲一組。現在我們有System.Collections.Concurrent中的所有這些易於使用的類,但在那裏沒有線程安全實現ISet<T>。嘆&hellip;

這就是說,如果你喜歡在存儲方面一個較爲有效的方法(這不一定是更快實施,根據對象的併發訪問模式),像這樣的工作,如一種替代方案:

private HashSet<string> lockSet; 
private readonly object _lock = new object(); 

public RefreshData(string key, Func<string, bool> cacheMissAction) 
{ 
    lock (_lock) 
    { 
     if (!lockSet.Add(key)) 
     { 
      return; 
     } 
    } 

    // if you are here means task was not already present 
    cacheMissAction(key); 

    lock (_lock) 
    { 
     lockSet.Remove(key); 
    } 
} 

在這種情況下,你確實需要的lock聲明,因爲HashSet<T>類本身不是線程安全的。這當然與您的原始實現非常相似,只是使用HashSet<T>的更多集合式語義來代替。

+0

很好的回答彼得! :) – Gopal 2015-06-26 08:30:49