0

I have a multi-threaded application that implements async methods. The application communicates over RabbitMq and at times needs to do a costly operation (BasicConsume on an IModel). I have a ConcurrentDictionary where I keep IModel (key) and it's IBasicConsumer (value) in order to avoid additional round-trips to the message broker.

On start-up, multiple executions reach the same section of the code where it looks if the consumer (value) exists. In the first ~40 cases the answer is no, but I don't want to do 40 BasicConsume, only one or perhaps two depending on the IModel (key) provided.

Using a lock statement introduces a performance hit that is not acceptable.

Currently, I'm solving the problem as follows

var consumerTcs = new TaskCompletionSouce<IBasicConsumer>();
if (_consumerDictionary.TryAdd(channel, consumerTcs))
{
    var newConsumer = new EventingBasicConsumer(channel);
    newConsumer.Model.BasicConsume(queue, false, newConsumer)
    consumerTcs.TrySetResult(newConsumer);
    return consumerTcs.Task;
}
else
{
    return _consumerDictionary[channel].Task;
}

That is

  1. Use a TaskCompletionSource (non-expencive) as the value of the dictionary
  2. Use TryAdd, and if successful do the expensive operation
  3. If unsuccessful, assume that the key exists and return the TaskCompletionSource.Task

However, I still create TaskCompletionSources that I don't use, which feels a bit ugly to me. Any suggestions to how I might solve this problem without the performance hit of a lock and without wasting clock cycles on creating objects that is not used?

Yuval Itzchakov
  • 146,575
  • 32
  • 257
  • 321
pardahlman
  • 1,394
  • 10
  • 22
  • How did you measure this unacceptable performance hit? – acelent Mar 15 '16 at 10:52
  • Also, this is what `GetOrAdd(TKey, Func)` is for, use it instead of the `TryAdd` and the possible race condition, i.e. if you ever remove values from the dictionary, `TryAdd` may return `false` but then obtaining the value may throw a `KeyNotFoundException`. – acelent Mar 15 '16 at 10:56
  • @acelent measured performance by running the code with nested tasks (20x outer and 50'000x inner) with a stopwatch. However `GetOrAdd` might be what I'm looking for! Thanks! – pardahlman Mar 16 '16 at 04:21
  • `GetOrAdd` does not work, as the `valueFactory` will be called until a `valueFactory` returns its value. This results in multiple unwanted roundtrips to the message broker. – pardahlman Mar 19 '16 at 19:02
  • You'd use `GetOrAdd` (with `Func<>`) to avoid creating many `TaskCompletionSouce`. It's nothing more than a `TryGet`, optionally followed by a `TryAdd` when there's no value, in essence reversing the logic you present. It may create a few unnecessary ones initially if there's a lot of contention, but the only way to create only one at all is with a lock around the block. Alas, with `GetOrAdd`, you have no direct notion if the returned object is the one you created or not, unless you set a captured variable, so a manual `TryGet`->`TryAdd` is simpler and does the same work. – acelent Mar 21 '16 at 11:03

0 Answers0