0

I'm trying to improve performance on a WPF app as my users are saddened by the fact that one part of the system, seems to have a performance issue. The part in question is a screen which shows logged in users. The slow part logs them out, by: scanning in their employee ref and finds their child control and removes it from the parent, i.e. logs them out. This currently uses a loop.

foreach (var userControl in UsersStackPanel.Children)
{
    if (userControl.Employee.EmpRef == employee.EmpRef)
    {                  
        // plus some DB stuff here
        UsersStackPanel.Children.Remove(userControl);                              
        break;
    }
}

but I've an alternative which does this,

var LoggedInUser = (UI.Controls.Generic.User)UsersStackPanel.Children
                       .OfType<FrameworkElement>()
                       .FirstOrDefault(e => e.Name == "EmpRef" + employee.EmpRef);
if (LoggedInUser != null)
{
    // some DB stuff here
    UsersStackPanel.Children.Remove(LoggedInUser);

}

I've timed both using the Stopwatch class but the results don't point to which is better, they both return 0 miliseconds. I am wondering if the DB part is the bottle neck, i just thought I'd start with the screen to improve things there as well as the DB updates. Any thoughts appreciated. Dan

abatishchev
  • 98,240
  • 88
  • 296
  • 433
ItsDanny
  • 165
  • 1
  • 11
  • 1
    0 milliseconds seems unlikely... i think your test wasn't quite up to snuff. – Timothy Groote Feb 19 '15 at 09:36
  • 1
    Also, you might want to check this : http://stackoverflow.com/questions/8214055/foreach-break-vs-linq-firstordefault-performance-difference basically, it means the LINQ version is slower. I doubt it will be that much slower that you would notice it in this particular app though. You're looking at micro-optimisations while there's an elephant in the room. – Timothy Groote Feb 19 '15 at 09:38
  • If you look at the source code for `FirstOrDefault`, you can see it uses a loop, too. http://referencesource.microsoft.com/#System.Core/System/Linq/Enumerable.cs,b7e5965cab68b1cf,references – Dennis_E Feb 19 '15 at 09:44
  • *Of course* it's the DB part. Unless you have thousands of children in that control, the looping is irrelevant. Before you start doing optimizations, you need to find out which part of the code actually needs the optimizations - profile the code and find where it's spending the most of its time. – Luaan Feb 19 '15 at 09:44
  • Most useful link that Dennis_E thanks. I felt LINQ must loop to some degree. I'll check the DB queries... – ItsDanny Feb 19 '15 at 10:02

1 Answers1

1

It seems to me that your second example should look more like this:

UI.Controls.Generic.User LoggedInUser = UsersStackPanel.Children
    .OfType<UI.Controls.Generic.User>()
    .FirstOrDefault(e => e.Employee.EmpRef == employee.EmpRef);
if (LoggedInUser != null)
{
    // some DB stuff here
    UsersStackPanel.Children.Remove(LoggedInUser);
}

But regardless, unless you have hundreds of thousands of controls in your StackPanel (and if you do, you have bigger fish to fry than this loop), the database access will completely swamp and make irrelevant any performance difference in the two looping techniques.

There's not enough context in your question to know for sure what the correct thing to do here is, but in terms of keeping the UI responsive, most likely what you'll want to wind up doing is wrapping the DB access in a helper method, and then execute that method as an awaited task, e.g. await Task.Run(() => DoSomeDBStuff()); That will let the UI thread (which is presumably the thread that executes the code you posted) continue working while the DB operations go on. When the DB stuff is done, your method will continue execution at the next statement, i.e. the call to the Remove() method.

Peter Duniho
  • 68,759
  • 7
  • 102
  • 136
  • Awesome thank Peter, I'll certainly pursue this line, anything that makes life a bit better for the users and I learn something new along the way. Cheers. – ItsDanny Feb 20 '15 at 08:41