Friday, August 6, 2010

ObservableCollection / DataGrid Memory Leaks

Ran into a BEAR of a memory leak today. Or, rather, one of our customer’s did … while investigating our PrismExplorer sample that demonstrates DevForce 2010 and Prism v.2.2 playing well together.

This post is a re-post of my exchange on our IdeaBlade Forum. I figure it’s worth repeating here should you run into it and not think to look on our forum.

The lesson, I hasten to add, is for all Silverlight developers. DevForce entities are not the cause. If you bind a Silverlight DataGrid to objects that implement INotifyPropertyChanged, this post is for you.

The takeaway is this: Don't reassign the ObservableCollection<T> bound to an ItemsSource; repopulate it!

An now … the story. First, the customer reports a problem

When ever I bind a datagrid or listbox via a ViewModel to an ObservableCollection(results) in a Callback method from a simple query … I end up consuming memory which will never be released, … not after disposing of the EntityManger, View, Viewmodel, etc. This behavior can be reproduced in your Prism Explorer when querying Customers, Orders, and back a few times. You can see this in Performance Monitor (perfmon) under process->private bytes where the count keeps climbing up.

Here is my response:
-------------------------------------------------------

You are (mostly) correct and oh ... how awful!

As I demonstrate below, memory is recovered when you clear the EntityManager. It would be recovered if you disposed of the EntityManager itself. I'm not sure why you didn't see these effects.

However, you are absolutely right that something horrible is happening. I spent much of the day researching it.

The only good part of the story is that it has nothing to do with DevForce. The cause is something in Silverlight, probably in the Silverlight DataGrid.

I will explain but it will be a bit of a journey leading ... ultimately ... to a sad-but-necessary workaround.

If you want to skip the journey and go right to the destination, the lesson for your code is:

Don't reassign the ObservableCollection<T> bound to an ItemsSource; repopulate it!

And now ... my sad tale ...

The main "ModelExplorer" view binds a Silverlight DataGrid.ItemsSource to the QueryResults property of the supporting ModelExplorerViewModel (the VM).

"QueryResults" is defined as IList but is always an ObservableCollection<T> in practice (henceforth written as "OC<T>").

The problem starts with the repeated resetting of the QueryResults in the ModelExplorerViewModel after every query.

A peculiarity of PrismExplorer (PE) is that the main "ModelExplorer" view could be asked to display objects of any type in the DataGrid. That's its purpose as a demo screen. Your grids typically only show one kind of object. But PE must work with arbitrary query result types.

Because those types change, the OC<T> must change to match type.

The obvious thing to do was craft a new OC<T> after every query ... one designed for the query results type, assign that new OC to the QueryResults, and raise PropertyChanged on "QueryResults". The DataGrid dutifully hears the call and refreshes its display.

Unfortunately, it consumes an extra 300K every single time. "300K" is not a misprint.

Here's the code with the garbage collection call and logging (aside: we now inject ILoggerFacade into the VM now so that we can write to the Visual Studio Output window).

    public IList QueryResults {
get { return _queryResults; }
set {
_queryResults = value;

Log("=== Total Memory = " + GC.GetTotalMemory(true));

RaisePropertyChanged("QueryResults", "QueryResultsCount");
SelectFirstQueryResultsItem();
}
}

If you add this instrumentation, query for "All Customers", and click the "Query" button repeatedly, the Output window will show that your memory consumption is climbing ~300K each time. Here are actual measurement.

Prism Explorer: "=== Total Memory = 3406212";
Prism Explorer: "=== Total Memory = 3914916";
Prism Explorer: "=== Total Memory = 4336668";
Prism Explorer: "=== Total Memory = 4756884";
Prism Explorer: "=== Total Memory = 5177356";
Prism Explorer: "=== Total Memory = 5782900";
Prism Explorer: "=== Total Memory = 6202860";
Prism Explorer: "=== Total Memory = 6616220";
// Cleared the EntityManager
Prism Explorer: "=== Total Memory = 3655320";
Prism Explorer: "=== Total Memory = 4076292";
Prism Explorer: "=== Total Memory = 4498300";
Prism Explorer: "=== Total Memory = 4918260";
Prism Explorer: "=== Total Memory = 5358912";
// Cleared the EntityManager
Prism Explorer: "=== Total Memory = 3655384";

Note that DevForce isn't going anywhere. You are issuing the same query repeatedly and DevForce satisfies it from the EntityManager cache. However, the PE ViewModel is building a new OC<Customer> each time and reassigning QueryResults. The DataGrid is rebinding the ItemsSource to the new OC<Customer> each time ... and chewing up memory each time.

If we comment out "_queryResult = value", DevForce and PE will continue to do their things ... DevForce may query the database or satisfy from cache if it can. PE will always build a new OC<T>, assign QueryResults with that OC<T>, raise PropertyChanged, stimulate the DataGrid to rebind to the QueryResults. Exactly the same code paths.

The only programmatic difference is that QueryResults always returns the exact same (empty) object.

The memory consumption goes flat ... as it should.

Of course it climbs modestly when you issue a query that fetches new data; that is to be expected. Press "Clear" to clear the EntityManager cache and memory is recovered.

This is proof enough for me that the memory leak is in Silverlight ... perhaps in the Silverlight DataGrid. Our entities are involved in some way ... that's why clearing the EntityManager cache allows the GC to recover memory. But the DataGrid is clearly chewing up memory at a fast clip without our adding any new entities to memory.

What could possibly take 300k? The entity data involved are miniscule (it's Northwind! 7 Employees ~90 customers). All entities combined do not weigh 300K.

The memory consumption grows by 300K when there are no new entities. Evidently Silverlight or OC is in a deadly embrace with the entities that prevents garbage collection (there are events involved). But even if so ... the numbers are staggering. We aren't adding 300K of new data each time we click the button. You can't write enough event handlers to consume 300K. The cost of an OC isn't 300K.

In my research, I tripped over an old WPF blog post (can't find it now) that said something about WPF creating completely new data and control templates for each OC. Now THAT is a fast way to burn memory. I'm betting that Silverlight is creating and caching a complete visual representation of the DataGrid and the associated entities each time I reset the ItemsSource with a new OC<T>!

I tried a number of things to shake up the binding ... anything to make it let go of whatever it was holding:


  • clear the _queryResults ( _queryResults.Clear() ) before re-assigning it with the new OC


  • clear the _queryResults and set it null before re-assigning it with the new OC


  • clear the _queryResults, set it null, raise PropertyChanged ... then assign with new OC

None of these attempts worked. Only pressing "Clear" worked.

What to do? The obvious answer is reuse the OC ... and that is the lesson for your application: Don't reassign the ItemsSource; repopulate it!

Of course OC<T> requires strongly typed contents. I tried OC<object> with the intention of simply clearing and refilling it with new results after each query. That executes without error. But it doesn't show anything on screen either ... pretty useless :-).

It actually worked if I use List<T> instead of OC<T>. But then I wouldn't have the benefits of ICollectionPropertyChanged and I'd have to manage adds, removals, clears by hand. Yuck.


The WorkAround

As a last resort, on each pass I check to see if the type of the fresh results matches the element type of the OC. If it does, I can re-populate the OC. If it does not match, I have to replace the ItemsSource with the new OC ... and pay the 300K tax.

If you always query for the same type, you'll be fine. You can change the query itself - there are numerous Customer and Employee queries in PE - without paying the tax. It's changing the result type that incurs the tax.

I tried to be clever and keep a dictionary of previously used OC types. If I was querying customers, I'd pull out the OC<Customer>; if I was querying employees, I'd pull out the OC<Employee>. That didn't help. The DataGrid doesn't remember OCs that it has seen before. Re-using old OCs didn't change its penchant for devouring another 300K

So all I can do is slow it down. If PE starts to blow, you can "Clear" the EntityManager and it will recover.

Here's the revised code (which will appear in the next PE release, minus the GC & Log calls)

    public IList QueryResults {
get { return _queryResults; }
set {

ResetQueryResults(value);

Log("=== Total Memory = " + GC.GetTotalMemory(true));

RaisePropertyChanged("QueryResults", "QueryResultsCount");
SelectFirstQueryResultsItem();
}
}


private void ResetQueryResults(IList results) {

if (null != _queryResults) _queryResults.Clear();
if (null == results 0 == results.Count) return;

var resultsType = TypeHelper.GetItemType(results);

if (_queryResultsElementType == resultsType) {
// Same results collection type; can refill it
foreach (var item in results) {
_queryResults.Add(item);
}
} else {
_queryResults = results;
_queryResultsElementType = resultsType;
}
}

private Type _queryResultsElementType;

Here are memory reports:

// Query for All Customers

Prism Explorer: "=== Total Memory = 3411896"
Prism Explorer: "=== Total Memory = 3425356"
Prism Explorer: "=== Total Memory = 3416064"
Prism Explorer: "=== Total Memory = 3416076"
Prism Explorer: "=== Total Memory = 3416076"
Prism Explorer: "=== Total Memory = 3429460"
Prism Explorer: "=== Total Memory = 3416076"
Prism Explorer: "=== Total Memory = 3415020"
// All Employees
Prism Explorer: "=== Total Memory = 4203344"
Prism Explorer: "=== Total Memory = 4212964"
Prism Explorer: "=== Total Memory = 4213000"
Prism Explorer: "=== Total Memory = 4213000"
Prism Explorer: "=== Total Memory = 4213000"
// Clear Entity Cache ... followed by All Employees
Prism Explorer: "=== Total Memory = 3706416"
Prism Explorer: "=== Total Memory = 3588660"
Prism Explorer: "=== Total Memory = 3593996"
// First Employee
Prism Explorer: "=== Total Memory = 3590928"
Prism Explorer: "=== Total Memory = 3604372"
Prism Explorer: "=== Total Memory = 3589676"
// Employees named Smith (there are none)
Prism Explorer: "=== Total Memory = 3605584"

// All Employees (again)

Prism Explorer: "=== Total Memory = 3757472"
Prism Explorer: "=== Total Memory = 3762120"
Prism Explorer: "=== Total Memory = 3770336"
Prism Explorer: "=== Total Memory = 3762144"

4 comments:

Anonymous said...

Similar to this issue with Windows Forms:

http://connect.microsoft.com/VisualStudio/feedback/details/236898/bindingcontext-should-cleanup-its-listmanagers-hashtable

Wouldn't hurt to report your problem, too.

Ward Bell said...

Good suggestion. I did just that on the Visual Studio bug reporting system. This is the report.

Anonymous said...

Ward,

Are you sure this issue is not in play:

http://forums.silverlight.net/forums/p/171739/387384.aspx#387384

Ward Bell said...

Thanks, Anonymous, ... nice find!

That link in the SilverlightForum aligns well with the scenario presented here. The autogenerating DataGrid is likely creating DataTemplates on the fly ("inline") and triggering this unwanted defect.

Looks like the root-cause will affect all controls, including 3rd party controls.

Tim Heuer writes in his July 30 comments "Right now we have a plan [for releasing a service pack that includes a fix to the DataTemplate leak problem] and are driving toward that plan which I've mentioned here as [expected] before end of Summer."

We'll just have to be patient.