ConcurrentDictionary: How do I add if value does not exist or condition fails?

Question

I am trying to implement extension method for ConcurrentDictionary that functionally gets the value from dictionary if key exists. If the key does not exists or the condition (in predicate) fails, it tries to get the value by calling valueFactory. Below is the method I have written.

public static TValue ConditionalGetOrAdd<TKey, TValue>(
    this ConcurrentDictionary<TKey, TValue> dictionary,
    TKey key,
    Func<TValue, bool> predicate,
    Func<TKey, TValue> valueFactory)
{
    TValue value = dictionary.GetOrAdd(key, valueFactory);

    if (!predicate(value))
    {
        value = valueFactory(key);
        dictionary[key] = value;
    }

    return value;
}

There are two issues here. I may call valueFactory twice in case the predicate returns false. This would be redundant operation. In addition, the entire method is not atomic. If one thread is inside the if block, the other thread will not wait for it to complete before calling GetOrAdd or predicate.

Is it possible to get this accomplished without taking lock on entire dictionary? In my case, the valueFactory is costly (makes an HTTP request internally), while predicate is quite cheap. Let me know if you have a solution. I am okay with modifying the entire method if it serves the purpose.

Why can't you move the predicate check to the top if, apparently, it is not dependent on whether it is inside the dictionary? — Jeroen Vannevel, Jul 15 '17 at 15:19
`predicate` will take the `value` that we get from dictionary. Unless I call `TryGet` or `GetOrAdd` first, I will not be able to know the value. Let me know if I did not understand the comment. — Jatin Sanghvi, Jul 15 '17 at 15:24
Ah I see now. Seems like you should just do a normal `TryGet()` instead of a `GetOrAdd()` since you don't want to add it? That way you will only call `valueFactory` at most once when the predicate fails. — Jeroen Vannevel, Jul 15 '17 at 15:31
https://msdn.microsoft.com/en-us/library/ee378664(v=vs.110).aspx — ProgrammingLlama, Jul 15 '17 at 15:33
Thanks. How do I prevent race condition then. I don't want while thread is calling `valueFactory`, other thread too get inside and to the same. @john, I will see if `AddOrUpdate` works for me. Give me some time. — Jatin Sanghvi, Jul 15 '17 at 15:35
I think `dictionary.AddOrUpdate(key, valueFactory, (k, v) => predicate(v) ? v : valueFactory(k))` should work for me. — Jatin Sanghvi, Jul 15 '17 at 15:39

score 2 · Answer 1 · answered Jul 15 '17 at 15:42

2

Thanks, @john for the suggestion. AddOrUpdate should work for me. Adding the new method body below:

return dictionary.AddOrUpdate(
     key,
     valueFactory,
     (k, v) => predicate(v) ? v : valueFactory(k));

I would keep this as an extension method instead of calling AddOrUpdate directly in the callers, as it would allow me reuse the valueFactory delegate at two places above.

answered Jul 15 '17 at 15:42

Jatin Sanghvi

1,928
1
22
33

2

Note that AddOrUpdate is [not atomic](https://learn.microsoft.com/en-us/dotnet/standard/collections/thread-safe/how-to-add-and-remove-items). The value factories may be called more than once for given key. – Mike Zboray Jul 15 '17 at 18:41
Woah! that's surprising. Thanks for the heads up. I checked this answer: https://stackoverflow.com/a/10487983/470119. It seems `AddOrUpdate` method will alternatively call `addValueFactory`/`updateValueFactory` until `TryAdd`/`TryUpdate` respectively succeeds on the factory-returned value. I will ensure that there's no side effect on calling `valueFactory` more than once. – Jatin Sanghvi Jul 15 '17 at 23:18
It is not surprising. It is well documented of all xORy style functions for ConcurrentDictionary<> class that any delegates are invoked outside of any locks held by the class itself. – Tanveer Badar Jul 30 '17 at 21:34

ConcurrentDictionary: How do I add if value does not exist or condition fails?

1 Answers1