3

Which of the following two pieces of code will perform better in different cases and why?

1.

private readonly ConcurrentDictionary<int, List<T>> _coll;
_coll.GetOrAdd(1, new List<T>());

This creates a new List on every call even when it is not needed (How much does this statement still matter if we pass the capacity as 0?).

2.

private readonly ConcurrentDictionary<int, List<T>> _coll;
_coll.GetOrAdd(1, (val) => new List<T>());

This only creates the List on demand, but has a delegate call.

Hele
  • 1,558
  • 4
  • 23
  • 39

2 Answers2

5

In terms of memory, the first way is going to cause an allocation every time, while the second will use a cached delegate object since it does not capture any variables. The compiler handles the generation of the cached delegate. There is no difference in the first case for capacity set to zero since the default constructor for List<T> uses an empty array on initialization, the same as an explicit capacity of 0.

In terms of execution instructions, they are the same when the key is found since the second argument is not used. If the key is not found, the first way simply has to read a local variable while the second way will have a layer of indirection to invoke a delegate. Also, looking into the source code, it appears that GetOrAdd with the factory will do an additional lookup (via TryGetValue) to avoid invoking the factory. The delegate could also potentially be executed multiple times. GetOrAdd simply guarantees that you see one entry in the dictionary, not that the factory is invoked only once.

In summary, the first way might be more performant if the key is typically not found since the allocation needs to happen anyway and there is no indirection via a delegate. However if the key is typically found, the second way is more performant because there are fewer allocations. For an implementation in a cache, you typically expect there to be lots of hits so if that is where this is, I would recommend the second way. In practice, the difference between the two depends on how sensitive the overall application is to allocations in this code path.

Also whatever implementation that is using this will likely need to implement locking around the List<T> that is returned since it is not thread-safe.

Mike Zboray
  • 39,828
  • 3
  • 90
  • 122
  • I used a `List` as an example, the actual collection in its place is thread-safe. Also, taking from Joseph's suggestion, does `() => Enumerable.Empty()` help with performance? – Hele Jan 21 '17 at 22:06
  • 1
    In addition it appears to depend on what framework version is being used (and whether or not new behavior was implemented), as was the case in .NET 4.5 where changes had some undesirable side effects. Refer to https://basildoncoder.com/blog/concurrentdictionary-getoradd-vs.html – Alex Jan 21 '17 at 22:09
  • @Hele Well if you are going to use `Enumerable.Empty()` then a delegate is not needed. `Enumerable.Empty()` just reads a static readonly field. However I'm not sure why you'd use a read-only interface like `IEnumerable`. I suspect it would make updates harder, but I'd need to see more of the code. – Mike Zboray Jan 21 '17 at 22:17
  • @Alex Interesting... I guess looking deeply at the code, GetOrAdd with factory includes some extra lookups. I might add that. – Mike Zboray Jan 21 '17 at 22:18
0

I can't imagine you'd see much of a difference in performance, unless you were working with an extremely large dataset. It would also depend on how likely each of your items are hit. Generics are extremely well optimised at the runtime level, and using a delegate results in an allocation either way.

My suggestion would be to use Enumerable.Empty<T>() as you'll be saving yourself an allocation on each array item.

Joseph Woodward
  • 9,191
  • 5
  • 44
  • 63