1

I have question about ConcurrencyDictionary in .NET C#. My app is going to be async (I try to do that :)).

I have some external devices, which send data to my core (C# .NET) via some TCPIP communication. I store the objects in values of ConcurrentDictionary for each device. I have some operations with that data, where I need to read it and sometimes change some in the object.

Now it looks good without deadlock (when I increase the number of external/simulated devices, it does not slow, but it can handle more messages in same time (and without data lose)

But: I am not sure if I'm using it correctly. I need to change some values inside of the object, call some functions and store all changes in the dict. All objects in the dict must be available to be read by other processes (I know during the "DoJob" other processes can have old values in dict until I will save value, but in my case it is ok). I just need to avoid blocking/locking other tasks and make it as fast as possible.

Which way is better:

1 way (i use it now):

var dict = new ConcurentDictionary<MyClass>(concurrencyLevel, initalCapacity);

private async Task DoJob(string myKey)
{
    MyClass myClass;
    MyClass myClassInitState;
    dict.TryGetValue(myKey, out myClass);
    dict.TryGetValue(myKey, out myClassInitState);

    var value = myClass.SomeValueToRead;
    myClass.Prop1 = 10;
    await myClass.DoSomeAnotherJob();

    dict.TryUpdate(myKey, myClass, myClassInitState);
}

2 way:

var dict = new ConcurentDictionary<MyClass>(concurrencyLevel, initalCapacity);

private async Task DoJob(string myKey)
{    
    var value = dict[myKey].SomeValueToRead;   
    dict[myKey].ChangeProp1(10);
    await dict[myKey].DoSomeAnotherJob();
}

The second way looks much more clear and simple. But I am not sure if I can do that because of async.

Will I block the other threads/tasks?

Which way will be faster? I expect first one, because inside of DoJob I do not work with dict, but with some copy of object and after all I will update the dict.

Does the reading of values directly (#2) could slow down the whole process?

Could other processes read last-actualised value from dict even during #2 way without any troubles?

What happen when I call:

dict[myKey].DoSomeAnotherJob();

It is awaitable, so it should not block the threads. But in fact it is called in shared dict in some its value.

jason.kaisersmith
  • 8,712
  • 3
  • 29
  • 51

2 Answers2

1

The thread-safe ConcurrentDictionary (as opposed to a plain old Dictionary) has nothing to do with async/await.

What this does:

await dict[myKey].DoSomeAnotherJob();

Is this:

var temp = dict[myKey];
await temp.DoSomeAnotherJob();

You do not need a ConcurrentDictionary in order to call that async method, dict can just as well be a regular Dictionary.

Also, assuming MyClass is a reference type (a class as opposed to a struct), saving its original reference in a temporary variable and updating the dictionary, as you do, is unnecessary. The moment after you called myClass.Prop1 = 10, this change is propagated to all other places where you have a reference to that same myClass instance.

You only want to call TryUpdate() if you want to replace the value, but you don't, as it's still the same reference - there's nothing to replace, both myClass and myClassInitState point to the same object.

The only reason to use a ConcurrentDictionary (as opposed to a Dictionary), is when the dictionary is accessed from multiple threads. So if you call DoJob() from different threads, that's when you should use a ConcurrentDictionary.

Also, when multithreading is involved, this is dangerous:

var value = dict[myKey].SomeValueToRead;   
dict[myKey].ChangeProp1(10);
await dict[myKey].DoSomeAnotherJob();

Because in the meantime, another thread could change the value for myKey, meaning you obtain a different reference each time you call dict[myKey]. So saving it in a temporary variable is the way to go.

Also, using the indexer property (myDict[]) instead of TryGetValue() has its own issues, but still no threading issues.

CodeCaster
  • 147,647
  • 23
  • 218
  • 272
0

Both ways are equal in effect. The first way uses methods to read from the collection and the second way uses an indexer to achieve the same. In fact the indexer internally invokes TryGetValue().

When invoking MyClass myClass = concurrentDictionary[key] (or concurrentDictionary[key].MyClassOperation()) the dictionary internally executes the getter of the indexer property:

public TValue this[TKey key]
{
  get
  {
    if (!TryGetValue(key, out TValue value))
    {
      throw new KeyNotFoundException();
    }
    return value;
  }     
  set
  {
    if (key == null) throw new ArgumentNullException("key");

    TryAddInternal(key, value, true, true, out TValue dummy);
  }
}

The internal ConcurrentDictionary code shows that

concurrentDictionary.TryGetValue(key, out value)

and

var value = concurrentDictionary[key]

are the same except the indexer will throw an KeyNotFoundException if the key doesn't exist.

From an consuming point of view the first version using TryGetValue enables to write more readable code:

// Only get value if key exists
if (concurrentDictionary.TryGetValue(key, out MyClass value))
{
  value.Operation();
}

vs

// Only get value if key exists to avoid an exception
if (concurrentDictionary.Contains(key))
{
  MyClass myClass = concurrentDictionary[key];
  myClass.Operation();
}

Talking about readability, your code can be simplified as followed:

private async Task DoJob(string myKey)
{
  if (dict.TryGetValue(myKey, out MyClass myClass))
  {
    var value = myClass.SomeValueToRead;
    myClass.Prop1 = 10;
    await myClass.DoSomeAnotherJob();    
  }
}

  • As async/await was designed to asynchronously execute operation on the UI thread, await myClass.DoSomeAnotherJob() won't block.
  • neither will TryGetValue nor this[] block other threads
  • both access variants execute at the same speed as they share the same implementation
  • dict[myKey].Operation()
    is equal to
    MyClass myclass = dict.[myKey]; myClass.Operation();.

    It's the same when
    GetMyClass().Operation()
    is equal to
    MyClass myClass = GetMyClass(); myClass.Operation();

  • your perception is wrong. Nothing is called "inside" the dictionary. As you can see from the internal code snippet dict[key] returns a value.

Remarks

ConcurrentDictionary is a way to provide thread-safe access to a collection. E.g., this means the thread that accesses the collection will always access a defined state of the collection. But be aware that the items itself are not thread-safe because they are stored in a thread-safe collection:

Consider the following method is executed by two threads simultaneously. Both threads share the same ConcurrentDictionary containing therefore shared objects.

// Thread 1
private async Task DoJob(string myKey)
{
  if (dict.TryGetValue(myKey, out MyClass myClass))
  {
    var value = myClass.SomeValueToRead;
    myClass.Prop1 = 10;

    await myClass.DoSomeLongRunningJob(); 

    // The result is now '100' and not '20' because myClass.Prop1 is not thread-safe. The second thread was allowed to change the value while this thread was reading it
    int result = 2 * myClass.Prop1; 
  }
}

// Thread 2
private async Task DoJob(string myKey)
{
  if (dict.TryGetValue(myKey, out MyClass myClass))
  {
    var value = myClass.SomeValueToRead;
    myClass.Prop1 = 50;
    await myClass.DoSomeLongRunningJob(); 
    int result = 2 * myClass.Prop1; // '100'
  }
}

Also ConcurrentDictionary.TryUpdate(key, newValue, comparisonValue) is the same like the following code:

if (dict.Contains(key))
{
  var value = dict[key];
  if (value == comparisonValue)
  {
    dict[key] = newValue;
  }
}

Example: Let's say the dictionary contains a numeric element at key "Amount" with a value of 50. Thread 1 only wants to modify this value if Thread 2 hasn't changed it in the meantime. Thread 2 value is more important (has precedence). You now can use the TryUpdate method to apply this rule:

if (dict.TryGetValue("Amount", out int oldIntValue))
{
  // Change value according to rule
  if (oldIntValue > 0)
  { 
    // Calculate a new value
    int newIntValue = oldIintValue *= 2;

    // Replace the old value inside the dictionary ONLY if thread 2 hasn't change it already
    dict.TryUpdate("Amount", newIntValue, oldIntValue);
  }
  else // Change value without rule
  {
    dict["Amount"] = 1;
  }
}
BionicCode
  • 1
  • 4
  • 28
  • 44
  • Thank you both for great answers. If I understand correctly, I do not worry now with async app, but if I expect the run some functions (for example I have HTTP API, EmbedIO in here which read/write some properties in MyClass) I will run in separated threads, I have to use lock inside of MyClass. – Lucie Svobodová Jan 23 '20 at 08:57
  • Correct. When `MyClass` has shared resources i.e. each thread can modify values of a property (either directly or throuogh a method), you have to synchronize this modification. Using a `lock` is one solution. There is [`Monitor`](https://docs.microsoft.com/en-us/dotnet/api/system.threading.monitor?view=netframework-4.8#examples) or [SemaphoreSlim](https://docs.microsoft.com/en-us/dotnet/api/system.threading.semaphoreslim?view=netframework-4.8#examples) and more. If you like to learn more take a look at this great source: [Threading in C# by Joseph Albahari](http://www.albahari.com/threading/). – BionicCode Jan 23 '20 at 09:38