2009-05-11 15 views
3

以下のクラスは、新しい "dataKey"が登録されるたびにイベントを発生させ、 "dataKey"が登録解除され、その "dataKey"がゼロである場合にイベントを発生させます。値のタイプと辞書の検​​索

このクラスはスレッドセーフであることを目指しており、できる限りパフォーマンスを向上させようとしています。

私の質問は、 Deregisterメソッドでは、値(_data [dataKey] = currentCountValue;)を更新すると何とか2番目のルックアップを削除できますか?

値がローカルスタックでのみ更新され、ディクショナリでは更新されないため、currentCountValue変数を単純に更新することはできません。

また、パフォーマンスの改善についてご意見をお聞かせください。私はロックを削除してCASの操作(インターロックされたメソッド)を使ってカウントを更新することはできないと思います。

/私はc#3.0を使用しています。

お時間をいただきありがとうございます。

public sealed class DataCounter 
{ 
    public event EventHandler NewKeyEvent; 
    public event EventHandler ZeroCountEvent; 
    private readonly Dictionary<string, int> _data = new Dictionary<string, int>(); 

    public void Register(string dataKey) 
    { 
     lock (_data) 
     { 
      if (_data.ContainsKey(dataKey)) 
      { 
       _data[dataKey]++; 
      } 
      else 
      { 
       _data.Add(dataKey, 1); 
       if (NewKeyEvent != null) NewKeyEvent(this, null); 
      } 
     } 
    } 

    public void Deregister(string dataKey) 
    { 
     lock (_data) 
     { 
      int currentCountValue; 
      if (_data.TryGetValue(dataKey, out currentCountValue)) 
      { 
       if (currentCountValue > 0) 
       { 
        currentCountValue--; 
        _data[dataKey] = currentCountValue; 
       } 

       if (currentCountValue == 0) 
       { 
        if (ZeroCountEvent != null) ZeroCountEvent(this, null); 
       } 
      } 
     } 
    } 
} 

答えて

2

考えてみましょう - インデクサーで「セット」する必要がない場合は、カウンターをクラスに移動できますか?

class CounterBox { 
    public int Count {get;set;} 
} 

次に、Dictionary<string,CounterBox>があります。辞書外にあるCountを更新できます。.Countがゼロの場合は、Remove(dataKey)のみを呼び出します。これは追加の参照解除を行いますが、インデクサー経由で割り当てる必要はありません。

どちらが速いのか:プロファイルする必要があります。以下のような

何か:

public sealed class DataCounter 
{ 
    private class CounterBox 
    { 
     public int Count { get; set; } 
    } 
    public event EventHandler NewKeyEvent; 
    public event EventHandler ZeroCountEvent; 
    private readonly Dictionary<string, CounterBox> _data 
     = new Dictionary<string, CounterBox>(); 

    public void Register(string dataKey) 
    { 
     lock (_data) 
     { 
      CounterBox box; 
      if (_data.TryGetValue(dataKey, out box)) 
      { 
       box.Count++; 
      } 
      else 
      { 
       _data.Add(dataKey, new CounterBox { Count = 1 }); 
       EventHandler handler = NewKeyEvent; 
       if (handler != null) handler(this, EventArgs.Empty); 
      } 
     } 
    } 

    public void Deregister(string dataKey) 
    { 
     lock (_data) 
     { 
      CounterBox box; 
      if (_data.TryGetValue(dataKey, out box)) 
      { 
       if (box.Count > 0) 
       { 
        box.Count--; 
       } 

       if (box.Count == 0) 
       { 
        EventHandler handler = ZeroCountEvent; 
        if (handler != null) handler(this, EventArgs.Empty); 
        _data.Remove(dataKey); 
       } 
      } 
     } 
    } 
} 
0

あなたのイベント処理は、スレッドセーフではありません。

// Execute this ... 
if (NewKeyEvent != null) 

// ... other threads remove all event handlers here ... 

// ... NullReferenceException here. 
    NewKeyEvent(this, null); 

次のようにしてください。

EventHandler newKeyEvent = this.newKeyEvent; 

if (newKeyEvent != null) 
{ 
    newKeyEvent(this, null); 
} 
0

あなたはイベントを起こしている方法に注意する必要があります(既に登録している人はスレッドセーフではありません)。

あなたはロック内でイベントハンドラを呼び出しています。スレッド自体は安全ではありませんが、データ構造を完全に停止させる危険性があります。あなたが呼び出すイベントハンドラで何が起きているのかを明らかに制御することはできないので、イベントハンドラ自体が長時間ブロックされたりフリーズしたりすると、ハンドラが戻るまで辞書がロックアウトされます。

ロックでは、制御できないメソッドを呼び出すべきではなく、実行時間が非決定的なメソッド(何らかの方法でメモリにアクセスしないもの)を決して呼び出すべきではない(SHOULD SHOULD)。そうした場合、コードがスレッドセーフであっても、ロックを無期限にブロックすることは脆弱です。

参照および参照解除では、呼び出しリストのコピーを持ち、ロックの外側を呼び出すか、デリゲート自身を外側に呼び出す必要があります(ダニエルのパターンを使用してください)。

関連する問題