Are delegates using an overly bad idea for performance?

Question

Consider the following code:

if (IsDebuggingEnabled) { instance.Log(GetDetailedDebugInfo()); } 

GetDetailedDebugInfo() can be an expensive method, so we only want to call it if we are in debug mode.

Now a cleaner alternative is code, for example:

 instance.Log(() => GetDetailedDebugInfo()); 

Where .Log () is defined as:

 public void Log(Func<string> getMessage) { if (IsDebuggingEnabled) { LogInternal(getMessage.Invoke()); } } 

My concern for performance, preliminary testing does not show that the second case will be especially expensive, but I do not want to encounter surprises when the load increases.

Oh, and please do not offer conditional compilation because this is not the case.

(PS: I wrote the code directly in the StackOverflow Ask a Question text box, so don't blame me if there are subtle errors and it doesn't compile, you get the point :)

Answer 1

I was hoping for some documentation regarding performance in such cases, but it seems that all I have is advice on how to improve my code ... No one seems to have read my PS - there are no glasses for you.

So, I wrote a simple test case:

  public static bool IsDebuggingEnabled { get; set; } static void Main(string[] args) { for (int j = 0; j <= 10; j++) { Stopwatch sw = Stopwatch.StartNew(); for (int i = 0; i <= 15000; i++) { Log(GetDebugMessage); if (i % 1000 == 0) IsDebuggingEnabled = !IsDebuggingEnabled; } sw.Stop(); Console.WriteLine(sw.ElapsedMilliseconds); } Console.ReadLine(); for (int j = 0; j <= 10; j++) { Stopwatch sw = Stopwatch.StartNew(); for (int i = 0; i <= 15000; i++) { if (IsDebuggingEnabled) GetDebugMessage(); if (i % 1000 == 0) IsDebuggingEnabled = !IsDebuggingEnabled; } sw.Stop(); Console.WriteLine(sw.ElapsedMilliseconds); } Console.ReadLine(); } public static string GetDebugMessage() { StringBuilder sb = new StringBuilder(100); Random rnd = new Random(); for (int i = 0; i < 100; i++) { sb.Append(rnd.Next(100, 150)); } return sb.ToString(); } public static void Log(Func<string> getMessage) { if (IsDebuggingEnabled) { getMessage(); } } 

The timing seems to be exactly the same between the two versions. I get 145 ms in the first case and 145 ms in the second case

Looks like I answered my question.

Answer 2

No, it should not have poor performance. In the end, you will only call it in debug mode, where performance is not at the forefront. In fact, you can remove the lambda and just pass the method name to remove the overhead of the unnecessary intermediate anonymous method.

Note that if you want to do this in Debug assemblies, you can add the [Conditional("DEBUG")] attribute to the log method.

Answer 3

There is a difference in performance. How important this is will depend on the rest of your code, so I would recommend profiling before proceeding with the optimization.

Having said this for your first example:

 if (IsDebuggingEnabled) { instance.Log(GetDetailedDebugInfo()); } 

If IsDebuggingEnabled is static readonly, then the check will be deleted as it knows that it will never change. This means that the above example will have zero performance impact if IsDebuggingEnabled is false, because after the JIT is completed, the code will be deleted.

 instance.Log(() => GetDetailedDebugInfo()); public void Log(Func<string> getMessage) { if (IsDebuggingEnabled) { LogInternal(getMessage.Invoke()); } } 

The method will be called every time after an instance of instance.Log. Which will be slower.

But before you spend time on this micro-optimization, you should profile your application or perform some performance tests to make sure that this is actually the bottleneck in your application.

Answer 4

You can also do this:

 // no need for a lambda instance.Log(GetDetailedDebugInfo) // Using these instance methods on the logger public void Log(Func<string> detailsProvider) { if (!DebuggingEnabled) return; this.LogImpl(detailsProvider()); } public void Log(string message) { if (!DebuggingEnabled) return; this.LogImpl(message); } protected virtual void LogImpl(string message) { .... } 

Answer 5

Call the delegate getMessage directly instead of invoking Invoke on it.

 if(IsDebuggingEnabled) { LogInternal(getMessage()); } 

You should also add a null check on getMessage.

Answer 6

Standard answers:

• If you need to do this, you must do it.
• Turn it over 10 ^ 9 times, look at the stopwatch, and this will tell you how many nanoseconds it takes.
• If your program is large, you may have more problems elsewhere.

Answer 7

I believe that delegates are creating a new thread, so you can be right about this while improving performance. Why not set up a test run, as Dove suggested, and carefully monitor the number of threads created by your application, for this you can use Process Explorer.

Hold on! I fixed it! Delegates only use streams when you use "BeginInvoke" ... so my comments above do not apply to how you use them.