A question recently came up over on the LINDUG about a particular method of implementing INotifyPropertyChanged. This interface has always been used for databinding, but has gained new importance with WPF and SilverLight binding. Writing code inline to each property can be tedious and the poster had a creative solution involving a static method to handle the logic. In his implementation, your properties look like this:
public int PublicInteger
{
get { return _privateInteger; }
set { PropertyHelper(this, ref _privateInteger, value, "PublicInteger",
PropertyChanged); }
}
We got to discussing the alternatives, and possible performance implications, and another poster pointed out that saving lines of code does not always equal better performance. So, out of curiosity, I thought I’d do a little test.
using System;
using System.Diagnostics;
using NUnit.Framework;
using System.ComponentModel;
namespace SpikeNotifyPropertyChanged
{
[TestFixture]
public class NotifyPropertyChangedTests
{
[Test]
public void CustomerNotifyWithInline_Should_Raise_PropertyChanged()
{
var target = new CustomerNotifyWithInline();
var propertyChangedWasRaised = false;
target.PropertyChanged += (s, e) => { propertyChangedWasRaised = true; };
target.CustomerName = "Test";
Assert.IsTrue(propertyChangedWasRaised);
}
public long CustomerNotifyWithInline_In_100000_Iterations()
{
var stopwatch = Stopwatch.StartNew();
for (var i = 0; i < 100000; i++)
{
CustomerNotifyWithInline_Should_Raise_PropertyChanged();
}
stopwatch.Stop();
Console.WriteLine("CustomerNotifyWithInline 100000 times took " + stopwatch.ElapsedTicks + " ticks.");
return stopwatch.ElapsedTicks;
}
[Test]
public void CustomerNotifyWithInstanceBase_Should_Raise_PropertyChanged()
{
var target = new CustomerNotifyWithInstanceBase();
var propertyChangedWasRaised = false;
target.PropertyChanged += (s, e) => { propertyChangedWasRaised = true; };
target.CustomerName = "Test";
Assert.IsTrue(propertyChangedWasRaised);
}
public long CustomerNotifyWithInstanceBase_In_100000_Iterations()
{
var stopwatch = Stopwatch.StartNew();
for (var i = 0; i < 100000; i++)
{
CustomerNotifyWithInstanceBase_Should_Raise_PropertyChanged();
}
stopwatch.Stop();
Console.WriteLine("CustomerNotifyWithInstanceBase 100000 times took " + stopwatch.ElapsedTicks + " ticks.");
return stopwatch.ElapsedTicks;
}
[Test]
public void CustomerNotifyWithStaticBase_Should_Raise_PropertyChanged()
{
var target = new CustomerNotifyWithStaticBase();
var propertyChangedWasRaised = false;
target.PropertyChanged += (s, e) => { propertyChangedWasRaised = true; };
target.CustomerName = "Test";
Assert.IsTrue(propertyChangedWasRaised);
}
public long CustomerNotifyWithStaticBase_In_100000_Iterations()
{
var stopwatch = Stopwatch.StartNew();
for (var i = 0; i < 100000; i++)
{
CustomerNotifyWithStaticBase_Should_Raise_PropertyChanged();
}
stopwatch.Stop();
Console.WriteLine("CustomerNotifyWithStaticBase 100000 times took " + stopwatch.ElapsedTicks + " ticks.");
return stopwatch.ElapsedTicks;
}
[Test]
public void CompareResults()
{
long instanceTime = 0;
long staticTime = 0;
long inlineTime = 0;
for (var i = 0; i < 10; i++)
{
instanceTime += CustomerNotifyWithInstanceBase_In_100000_Iterations() ;
staticTime += CustomerNotifyWithStaticBase_In_100000_Iterations();
inlineTime += CustomerNotifyWithInline_In_100000_Iterations();
}
instanceTime = instanceTime/10;
staticTime = staticTime/10;
inlineTime = inlineTime/10;
Console.WriteLine("Average instance time: " + instanceTime);
Console.WriteLine("Average static time: " + staticTime);
Console.WriteLine("Average inline time: " + inlineTime);
}
}
public class CustomerNotifyWithStaticBase : NotifyWithStaticBase
{
private string customerName;
public string CustomerName
{
get { return customerName; }
set
{
SetValue(this, ref customerName, value, "CustomerName", GetHandler());
}
}
}
public class NotifyWithStaticBase : INotifyPropertyChanged
{
public event PropertyChangedEventHandler PropertyChanged;
public PropertyChangedEventHandler GetHandler()
{
return this.PropertyChanged;
}
protected static void SetValue<T>(object sender, ref T field, T value, string propertyName, PropertyChangedEventHandler handler) where T : class
{
if (field != value)
{
field = value;
if (handler != null)
handler(sender, new PropertyChangedEventArgs(propertyName));
}
}
}
public class CustomerNotifyWithInstanceBase : NotifyWithInstanceBase
{
private string customerName;
public string CustomerName
{
get { return customerName; }
set
{
SetValue(ref customerName, value, "CustomerName");
}
}
}
public class NotifyWithInstanceBase : INotifyPropertyChanged
{
public event PropertyChangedEventHandler PropertyChanged;
protected void SetValue<T>(ref T field, T value, string propertyName) where T: class
{
if (field != value)
{
field = value;
var handler = PropertyChanged;
if (handler != null)
handler(this, new PropertyChangedEventArgs(propertyName));
}
}
}
public class CustomerNotifyWithInline : INotifyPropertyChanged
{
private string customerName;
public event PropertyChangedEventHandler PropertyChanged;
public string CustomerName
{
get { return customerName; }
set
{
if (customerName != value)
{
customerName = value;
OnPropertyChanged("CustomerName");
}
}
}
private void OnPropertyChanged(string propertyName)
{
var handler = PropertyChanged;
if (handler != null)
handler(this, new PropertyChangedEventArgs(propertyName));
}
}
}
This code implements INotifyPropertyChanged in three different ways.
- Inline- with all of the logic inside the property setter, and no base class.
- StaticBase – with a static method inside a base class, similar to the poster.
- InstanceBase – with an instance method inside a base class
It then tests each with 10 iterations of 100000 calls to set CustomerName, and averages the number of ticks. Unfortunately, the test is inconclusive at this point. The times are just too close. Plus, with each run of the test, a different method is fastest, which means that some internals to .NET, such as garbage collection or some other factor, are skewing the results:
Average instance time: 84166
Average static time: 82924
Average inline time: 80326
Average instance time: 78257
Average static time: 78422
Average inline time: 75133
All is not lost, though. We can use NDepend to examine the code. In terms of IL Instructions:
- Inline is 63 IL Instructions, and has no base class.
- StaticBase is 21 for the subclass and 54 for the base class, or 75 total IL instructions.
- InstanceBase is 19 for the subclass and 50 for the base class, or 69 total IL instructions.
So, strictly speaking, inline has fewer instructions total, but using a base class makes more sense if you have more than one class inheriting from it. As we added classes, InstanceBase would be a narrow winner. It also has the lowest cyclomatic complexity by one or two points.
But there’s still one more tool we can throw at this. Visual studio’s .NET built-in profiler instruments or samples the app and measures the total time an app spends inside a particular method. As with the timed tests, the results varied, but generally favored the static approach.
I’m not sure my tests are conclusive at all, but the static approach may have a slight performance advantage. In retrospect, this makes sense, as Microsoft Design Guidelines recommend marking methods static when possible:
After you mark the methods as static, the compiler will emit non-virtual call sites to these members. Emitting non-virtual call sites will prevent a check at runtime for each call that ensures that the current object pointer is non-null. This can result in a measurable performance gain for performance-sensitive code.
Of course, this doesn’t necessarily mean everything should be static- in this case the static approach necessitates some additional code that nearly erases any performance benefits. In both cases, the use of ‘ref’ is also questioned by MS design guidelines. In the end, there are probably more significant bottlenecks to worry about, but it’s always interesting to dig into stuff like this. My recommendation: create a base class to help reduce repetitive code with an approach that doesn’t use ‘ref’, or use something like CSLA.
No comments:
Post a Comment