views:

149

answers:

2

The problem

I've been struggling with this partiular problem for two days now and just run out of ideas. A little... background: we have a WinForms app that needs to access a database, construct a list of related in-memory objects from that data, and then display on a DataGridView. Important point is that we first populate an app-wide cache (List), and then create a mirror of the cache local to the form on which the DGV lives (using List constructor param).

Because fetching the data takes a good few seconds (DB sits on a LAN server) to load, we decided to use a BackgroundWorker, and only refresh the DGV once the data is loaded. However, it seems that doing the loading via a BGW results in some memory leak... or an error on my part. When loaded using a blocking method call, the app consumes about 30MB of RAM; with a BGW this jumps to 80MB! While it may not seem as much anyway, our clients are not too happy about it.

Relevant code

Form

private void MyForm_Load(object sender, EventArgs e)
{
    MyRepository.Instance.FinishedEvent += RefreshCache;
}
private void RefreshCache(object sender, EventArgs e)
{
    dgvProducts.DataSource = new List<MyDataObj>(MyRepository.Products);
}

Repository

private static List<MyDataObj> Products { get; set; }
public event EventHandler ProductsLoaded;

public void GetProductsSync()
{
    List<MyDataObj> p;

    using (MyL2SDb db = new MyL2SDb(MyConfig.ConnectionString))
    {
        p = db.PRODUCTS
        .Select(p => new MyDataObj {Id = p.ID, Description = p.DESCR})
        .ToList();
    }

    Products = p;

    // tell the form to refresh UI
    if (ProductsLoaded != null)
        ProductsLoaded(this, null);

}

public void GetProductsAsync()
{
    using (BackgroundWorker myWorker = new BackgroundWorker())
    {
        myWorker.DoWork += delegate
        {
            List<MyDataObj> p;
            using (MyL2SDb db = new MyL2SDb(MyConfig.ConnectionString))
            {
                p = db.PRODUCTS
                .Select(p => new MyDataObj {Id = p.ID, Description = p.DESCR})
                .ToList();
            }

            Products = p;
        };

        // tell the form to refresh UI when finished
        myWorker.RunWorkerCompleted += GetProductsCompleted;
        myWorker.RunWorkerAsync();
    }
}

private void GetProductsCompleted(object sender, RunWorkerCompletedEventArgs e)
{
    if (ProductsLoaded != null)
        ProductsLoaded(this, null);
}

End!

GetProductsSync or GetProductsAsync are called on the main thread, not shown above. Could it be that the GarbageCollector just gets lost with two threads? Or is it the task manager that shows incorrect values?

Will be greateful for any responses, suggestions, criticism.

A: 

I'm not entirely sure if this will help, but in the Async method you could change this:

List<MyDataObj> p;
using (MyL2SDb db = new MyL2SDb(MyConfig.ConnectionString))
{
    p = db.PRODUCTS
    .Select(p => new MyDataObj {Id = p.ID, Description = p.DESCR})
    .ToList();
}

Products = p;

to this:

using (MyL2SDb db = new MyL2SDb(MyConfig.ConnectionString))
{
    Products = db.PRODUCTS
    .Select(p => new MyDataObj {Id = p.ID, Description = p.DESCR})
    .ToList();
}

I don't think you need the extra list variable in there. That might be why? You were creating a whole extra list? Either way, this is also a little cleaner looking :)

SkippyFire
This cannot be causing the problem; this is an extra reference to the same list, and does not copy any objects.
SLaks
Hmmm... I just realized you're doing the same thing in the synchronous code... so it's probably not that. But maybe change the code anyway to see if it helps both?
SkippyFire
Thanks for the input but tend to agree with SLaks, it's definitely not the issue here. I prefer keep a local variable, assign it inside the using, and do whatever else needs to be done outside of the using. That way I don't have eg. a return inside it, which imho makes for a lil more readable/understandable code. But that's just a personal choice :-)
Dav
+1  A: 

Funny that - followed Henk's advice and used a real profiler (.Net Memory Profiler) rather than task manager.

While mem use numbers are almost identical, the expected number of MyDataObj instances equal to the expected (db) in both sync and async cases, Virtual Memory and Heap sizes also very close... still something curious is going on. There's always a 1.5MB difference that stems from a call to VirtualAlloc() by ntdll. About 1MB out of this comes from DllUnregisterServerInternal(), which takes up 18.7MB in the async case (vs. 17.7MB ). Most-of-the-rest comes from CoUninitializeEE() that does get called in the async version, but isn't called by the sync app (?). I know, this is digging deep in mud - apologies. The above 1.5MB is the only real difference I could find - just my wild guess that it could be a sign of something else going on.

The real question is: why does the task manager show wildly different numbers? Does it not handle BackgroundWorkers well? Have you ever come across such a massive difference (30MB vs 80MB)?

Dav
Dav, as far as gauging mem usage by the TaskManager goes 50MB is a tiny difference. But in real world terms it is also small, and Memory usage under a modern OS is a very complex affair.
Henk Holterman
Well, guess I shouldn't have put it 'massive difference' - what I meant was that 80MB is over twice as much as 30MB, not that an app consuming 80MB is a resource hog. Perhaps you're just right - again, and it's up to us to educate clients about resource consumption estimations in a managed environment. Thanks for all your input Henk, and keeping your head cool. (Would still love to bring this up to MS task manager team but that's another story!)
Dav