ChatGPT解决这个技术问题 Extra ChatGPT

Is there a downside to adding an anonymous empty delegate on event declaration?

I have seen a few mentions of this idiom (including on SO):

// Deliberately empty subscriber
public event EventHandler AskQuestion = delegate {};

The upside is clear - it avoids the need to check for null before raising the event.

However, I am keen to understand if there are any downsides. For example, is it something that is in widespread use and is transparent enough that it won't cause a maintenance headache? Is there any appreciable performance hit of the empty event subscriber call?


C
Community

Instead of inducing performance overhead, why not use an extension method to alleviate both problems:

public static void Raise(this EventHandler handler, object sender, EventArgs e)
{
    if(handler != null)
    {
        handler(sender, e);
    }
}

Once defined, you never have to do another null event check again:

// Works, even for null events.
MyButtonClick.Raise(this, EventArgs.Empty);

See here for a generic version: stackoverflow.com/questions/192980/…
Exactly what my helper trinity library does, incidentally: thehelpertrinity.codeplex.com
Doesn't this just move the thread problem of the null check into your extension method?
No. Handler is passed into the method, at which point, that instance can't be modified.
@PatrickV No, Judah is right, the parameter handler above is a value parameter to the method. It won't change while the method is running. For that to occur, it would need to be a ref parameter (obviously it is not allowed for a parameter to have both the ref and the this modifier), or a field of course.
K
Kent Boogaart

For systems that make heavy use of events and are performance-critical, you will definitely want to at least consider not doing this. The cost for raising an event with an empty delegate is roughly twice that for raising it with a null check first.

Here are some figures running benchmarks on my machine:

For 50000000 iterations . . .
No null check (empty delegate attached): 530ms
With null check (no delegates attached): 249ms
With null check (with delegate attached): 452ms

And here is the code I used to get these figures:

using System;
using System.Diagnostics;

namespace ConsoleApplication1
{
    class Program
    {
        public event EventHandler<EventArgs> EventWithDelegate = delegate { };
        public event EventHandler<EventArgs> EventWithoutDelegate;

        static void Main(string[] args)
        {
            //warm up
            new Program().DoTimings(false);
            //do it for real
            new Program().DoTimings(true);

            Console.WriteLine("Done");
            Console.ReadKey();
        }

        private void DoTimings(bool output)
        {
            const int iterations = 50000000;

            if (output)
            {
                Console.WriteLine("For {0} iterations . . .", iterations);
            }

            //with anonymous delegate attached to avoid null checks
            var stopWatch = Stopwatch.StartNew();

            for (var i = 0; i < iterations; ++i)
            {
                RaiseWithAnonDelegate();
            }

            stopWatch.Stop();

            if (output)
            {
                Console.WriteLine("No null check (empty delegate attached): {0}ms", stopWatch.ElapsedMilliseconds);
            }


            //without any delegates attached (null check required)
            stopWatch = Stopwatch.StartNew();

            for (var i = 0; i < iterations; ++i)
            {
                RaiseWithoutAnonDelegate();
            }

            stopWatch.Stop();

            if (output)
            {
                Console.WriteLine("With null check (no delegates attached): {0}ms", stopWatch.ElapsedMilliseconds);
            }


            //attach delegate
            EventWithoutDelegate += delegate { };


            //with delegate attached (null check still performed)
            stopWatch = Stopwatch.StartNew();

            for (var i = 0; i < iterations; ++i)
            {
                RaiseWithoutAnonDelegate();
            }

            stopWatch.Stop();

            if (output)
            {
                Console.WriteLine("With null check (with delegate attached): {0}ms", stopWatch.ElapsedMilliseconds);
            }
        }

        private void RaiseWithAnonDelegate()
        {
            EventWithDelegate(this, EventArgs.Empty);
        }

        private void RaiseWithoutAnonDelegate()
        {
            var handler = EventWithoutDelegate;

            if (handler != null)
            {
                handler(this, EventArgs.Empty);
            }
        }
    }
}

You're kidding, right? The invocation adds 5 nanoseconds and you're warning against doing it? I can't think of a more unreasonable general optimization than that.
Interesting. According to your findings it is faster to check for null and call a delegate than just to call it without the check. Doesn't sound right to me. But anyway this is such a small difference that I don't think it is noticeable in all but the most extreme cases.
Brad, I specifically said for performance-critical systems that make heavy use of events. How is that general?
You're talking about performance penalties when calling empty delegates. I ask you: what happens when somebody actually subscribes to the event? You should worry about the performance of the subscribers not of the empty delegates.
There is a misleading fluke in the output benchmark because two different objects are used for the warmup and the real run. Thus, the "warmup" is not effective on the second call, since it just warmed up the second object. The fluke is due to the first call to the empty delegates that has to be created, that is way longer than a simple call. It'd be nice if you could correct the results by using the same object for both call. :)
M
Maurice

The only downside is a very slight performance penalty as you are calling extra empty delegate. Other than that there is no maintenance penalty or other drawback.


If the event would otherwise have exactly one subscriber (a very common case), the dummy handler will make it have two. Events with one handler are handled much more efficiently than those with two.
M
Marc Gravell

If you are doing it a /lot/, you might want to have a single, static/shared empty delegate that you re-use, simply to reduce the volume of delegate instances. Note that the compiler caches this delegate per event anyway (in a static field), so it is only one delegate instance per event definition, so it isn't a huge saving - but maybe worthwhile.

The per-instance field in each class will still take the same space, of course.

i.e.

internal static class Foo
{
    internal static readonly EventHandler EmptyEvent = delegate { };
}
public class Bar
{
    public event EventHandler SomeEvent = Foo.EmptyEvent;
}

Other than that, it seems fine.


It seems from the answer here: stackoverflow.com/questions/703014/… that the compiler does the optimization to a single instance already.
C
Christopher Bennage

It is my understanding that the empty delegate is thread safe, whereas the null check is not.


Neither of the patterns make the event thread safe as stated in the answer here: stackoverflow.com/questions/10282043/…
@Beachwalker That answer is misleading. Delegate calls are safe, as long as the delegate is not null. stackoverflow.com/a/6349163/2073670
S
Sergey Kalinichenko

There is no meaningful performance penalty to talk about, except, possibly, for some extreme situations.

Note, however, that this trick becomes less relevant in C# 6.0, because the language provides an alternative syntax to calling delegates that may be null:

delegateThatCouldBeNull?.Invoke(this, value);

Above, null conditional operator ?. combines null checking with a conditional invocation.


"the language provides an alternative syntax..." that hides the complexity and potential multithreading issues presented by the original language design decision (that basically requires always wrapping delegates in a null check.) I suggest that this syntax is an antipattern and was added to supports poor coding habits that have become normalized.
@tekHedd Going a level deeper, we should question the introduction of the null reference itself. Sir Charles Antony Richard Hoare, who invented the unfortunate thing, has called it his billion-dollar mistake. Personally, I think he is being way too modest with his estimate: today, the cost of that mistake is probably closing on a trillion-dollar mark. That's why I think that the decision to move C# toward the opt-in nullability is a step in the right direction.
S
Scott P

I would say it's a bit of a dangerous construct, because it tempts you to do something like :

MyEvent(this, EventArgs.Empty);

If the client throws an exception, the server goes with it.

So then, maybe you do:

try
{
  MyEvent(this, EventArgs.Empty);
}
catch
{
}

But, if you have multiple subscribers and one subscriber throws an exception, what happens to the other subscribers?

To that end, I've been using some static helper methods that do the null check and swallows any exception from the subscriber side (this is from idesign).

// Usage
EventHelper.Fire(MyEvent, this, EventArgs.Empty);


public static void Fire(EventHandler del, object sender, EventArgs e)
{
    UnsafeFire(del, sender, e);
}
private static void UnsafeFire(Delegate del, params object[] args)
{
    if (del == null)
    {
        return;
    }
    Delegate[] delegates = del.GetInvocationList();

    foreach (Delegate sink in delegates)
    {
        try
        {
            sink.DynamicInvoke(args);
        }
        catch
        { }
    }
}

Not to nitpick, but dont you thing if (del == null) { return; } Delegate[] delegates = del.GetInvocationList(); is a race condition candidate?
Not quite. Since delegates are value types, del is actually a private copy of the delegate chain which is accessible only to the UnsafeFire method body. (Caveat: This fails if UnsafeFire gets inlined, so you'd need to use the [MethodImpl(MethodImplOptions.NoInlining)] attribute to inure against it.)
1) Delegates are reference types 2) They are immutable, so it's not a race condition 3) I don't think inlining does change the behavior of this code. I'd expect that the parameter del becomes a new local variable when inlined.
Your solution to 'what happens if an exception is thrown' is to ignore all of the exceptions? This is why I always or almost always wrap all event subscriptions in a try (using a handy Try.TryAction() function, not an explicit try{} block), but I don't ignore the exceptions, I report them...
C
Community

Instead of "empty delegate" approach one can define a simple extension method to encapsulate the conventional method of checking event handler against null. It is described here and here.


C
Community

One thing is missed out as an answer for this question so far: It is dangerous to avoid the check for the null value.

public class X
{
    public delegate void MyDelegate();
    public MyDelegate MyFunnyCallback = delegate() { }

    public void DoSomething()
    {
        MyFunnyCallback();
    }
}


X x = new X();

x.MyFunnyCallback = delegate() { Console.WriteLine("Howdie"); }

x.DoSomething(); // works fine

// .. re-init x
x.MyFunnyCallback = null;

// .. continue
x.DoSomething(); // crashes with an exception

The thing is: You never know who will use your code in which way. You never know, if in some years during a bug fix of your code the event/handler is set to null.

Always, write the if check.

Hope that helps ;)

ps: Thanks for the performance calculation.

pps: Edited it from a event case to and callback example. Thanks for the feedback ... I "coded" the example w/o Visual Studio and adjusted the example I had in mind to an event. Sorry for the confusion.

ppps: Do not know if it still fits to the thread ... but I think it is an important principle. Please also check another thread of stackflow


x.MyFunnyEvent = null; <- That doesn't even compile(outside of the class). The point of an event is only supporting += and -=. And you can't even do x.MyFunnyEvent-=x.MyFunnyEvent outside of the class since the event getter is quasi protected. You can only break the event from the class itself(or from a derived class).
you are right ... true for events ... had a case with a simple handler. Sorry. I will try to edit.
Of course if your delegate is public that would be dangerous because you never know what the user will do. However, if you set the empty delegates on a private variable and you handle the += and -= yourself, that won't be a problem and the null check will be thread safe.