5

Today, I came across an interesting method of implementing the INotifyPropertyChanged interface. Instead of passing a string name of the property changed, or a lambda expression, we can simply call RaisePropertyChanged(); from the setter, mark that the call is parameterless. This is the code in the RaisePropertyChanged() method:

public virtual void RaisePropertyChanged()
{
    var frames = new System.Diagnostics.StackTrace();
    for (var i = 0; i < frames.FrameCount; i++)
    {
        var frame = frames.GetFrame(i).GetMethod() as MethodInfo;
        if (frame != null)
            if (frame.IsSpecialName && frame.Name.StartsWith("set_"))
            {
                RaisePropertyChanged(frame.Name.Substring(4));
                return;
            }
    }
    throw new InvalidOperationException("NotifyPropertyChanged() can only by invoked within a property setter.");
}

And this is a property that will notify its dependants of its change:

public string MyProperty
{
    get { return _myField; }
    set
    {
        _myField= value;
        RaisePropertyChanged();
    }
}

While I find this approach interesting, I think the performance penalty could be serious in case the property changed often... or if every property in our application used this approach to notify of its change.

I'd like to hear your opinions. (there is no longer the community-wiki checkbox?) Would this approach be very inefficient?

Source: Article where this approach is presented

Peter Perháč
  • 20,434
  • 21
  • 120
  • 152
  • Check Pete Brown`s blog as well on this question : http://10rem.net/blog/2010/12/16/strategies-for-improving-inotifypropertychanged-in-wpf-and-silverlight?utm_source=feedburner&utm_medium=feed&utm_campaign=Feed%3A+PeteBrown+%28Pete+Brown%27s+Blog%29 . It seems Reflection is hungry enough on ressources to be cautious about using it in something like INotifiyPropertyChanged, that will be called often. But I dont have any personnal experience about comparing the two methods on a scale that would let me recommend a method or another. – Matthieu Dec 23 '10 at 16:35

5 Answers5

8

I just tested it using this code. (Note that I circumvented the limitation pointed out in Wim Coenen's answer using the MethodImpl attribute. I have my doubts as to whether this is a surefire workaround.)

Results:

Raised event using reflection 1000 times in 25.5334 ms.
Raised event WITHOUT reflection 1000 times in 0.01 ms.

So you can see, the solution involving the stack trace has about 2,500 times the cost of the "normal" solution.

That's the proportional answer, anyway. I personally dislike this idea (clever though it may be) for reasons quite beyond performance issues alone; but, obviously, it's your call.


Edit: For the record, I felt compelled to write a blog post about this—in particular, about the fact that some developers would be tempted to use an approach like this in spite of the performance hit.

Whether you agree with my feelings on the subject or not (I realize that the performance hit is small in absolute terms), I feel that the real killing blow to this idea is that, for it to be even remotely robust, it is necessary to decorate every property setter from which you intend to call RaisePropertyChanged with the MethodImpl attribute, passing MethodImplOptions.NoInlining... which, right there, negates whatever typing savings you otherwise gained.

So you're left with a net loss in development time (by however many seconds it took you to type out the whole MethodImpl attribute part), plus a performance penalty. Very little reason to go this route, if you ask me.

Community
  • 1
  • 1
Dan Tao
  • 125,917
  • 54
  • 300
  • 447
  • 1
    +1 I dislike the reflection approach as it uses reflection to find out what you already know. You already know what's calling `RaisePropertyChanged`; it's a couple of lines away in the source code. – Tim Robinson Dec 23 '10 at 17:24
2

Yeesh, this seems like a lot of work, it'll be slow, and you run the risk of getting the method inlined.

If you want to do this, I'd suggest putting a [MethodImplAttribute] that says not to inline.

My suggestion would be to use a DynamicProxy instead as it'll be much easier and much faster than this approach, the only downside is you must specify the method as virtual. For instance in my work in progress proxy I specify a metaprogramming definition and bind it for my properties, here is my NotifyPropertyChanged interceptor.

    private static void SetterInterceptor<T, TProperty>(ProxiedProperty<T, TProperty> property, T target, TProperty value) where T:class,IAutoNotifyPropertyChanged
    {
        TProperty oldValue;
        if (!EqualityComparer<TProperty>.Default.Equals(oldValue = property.GetMethod.CallBaseDelegate(target), value))
        {
            property.SetMethod.CallBaseDelegate(target, value);
            target.OnPropertyChanged(new PropertyChangedEventArgs(property.Property.Name));
        }
    }

Then it's just a foreach loop across the properties I'm interested in with calls to Delegate.CreateDelegate to perform the binding.

Michael B
  • 7,512
  • 3
  • 31
  • 57
0

I have no clear proofs, but I believe yes, it would be very costly, especially if every property does this.

Michael Sagalovich
  • 2,539
  • 19
  • 26
0

Performance doesn't matter, because the linked article states that it doesn't really work:

Apparently this is a method that has been tried in the past. As Sergey Barskiy pointed out, the JITter will likely inline the method and break the stack frame.

Wim Coenen
  • 66,094
  • 13
  • 157
  • 251
  • i originally read about this on a website written in Czech and the guy there basically stated that he's tested this approach and it works fine. I believe it wouldn't work in *every* circumstance... but still interesting – Peter Perháč Dec 23 '10 at 16:47
  • If the method is public, there is no reason why the stack frame would be changed/inlined. Plus you still can add an additional MethodImplAttribute(MethodImplOptions.NoInlining). Note that does not mean I endorse the method :-) – Simon Mourier Dec 23 '10 at 17:10
  • @Simon Mourier: It's the property setter that gets inlined. If you look at the code I linked to in my answer, the `MethodImpl` attribute actually *was* necessary; before I added that part the method was broken as a result of the inlining. – Dan Tao Dec 23 '10 at 17:14
0

A different approach that I like better is here:

You can get the property name using reflection on a lambda function that calls the property getter.

How to raise PropertyChanged event without using string name

Community
  • 1
  • 1
NotDan
  • 31,709
  • 36
  • 116
  • 156