3

What is the most efficient way to traverse a collection/IEnumeration in C#. I have a list which contains almost 1100 objects. almost 10 of those objects, inturn contain 1000 subobjects (of same type). Traversal to this list takes almost 5-6 seconds. Here is my code:

foreach (Parameter par in this.AllParameters) //this.AllParameters is Generic.List type
{
    foreach (Parameter subPar in par.WrappedSubParameters)
    {
        subPar.IsSelected = false;
    }
    par.IsSelected = false;
}

Is there a way to optimize this code so that it is fast enough, not taking 5-6 seconds?

John Saunders
  • 160,644
  • 26
  • 247
  • 397
Irfan
  • 2,713
  • 3
  • 29
  • 43
  • 5
    I suggest that you first run [a profiler](http://stackoverflow.com/q/3927/87698) over your code. Time might be lost in the `AllParameters` property, in the `WrappedSubParameters` property, in the `IsSelected` setter, in one of the enumerators, etc. (Oh, and if you have the time, Eric Lippert's [series on common benchmarking mistakes](http://tech.pro/blog/1293/c-performance-benchmark-mistakes-part-one) is worth a read.) – Heinzi Jul 17 '13 at 15:54
  • I wouldn't expect roughly one million writes, like this, to take 5-6 seconds. Are you timing in a debug build? In a release build outside VS, I'd expect this to be a bit quicker than that on most systems, unless there is something else happening, such as `IsSelected` being data bound, etc. – Reed Copsey Jul 17 '13 at 15:56
  • Yes @ReedCopsey, IsSelected is bound with selection property of DataGrid. Could this be the reason for time-taking loop? – Irfan Jul 17 '13 at 15:59
  • @Irfan Yes - the data binding will slow things down. – Reed Copsey Jul 17 '13 at 16:20
  • 1
    You probably need to inform the grid of massive changes, such as something like `BeginUpdate` or whatnot to make it sit out all the minor updates and do a complete update at the end. Many grids have such things, not sure about DataGrid. – Lasse V. Karlsen Jul 17 '13 at 16:23

2 Answers2

4

The loops, as written, are likely one of the fastest options.

Since this is all in memory, and each write operation appears to be on a separate instance (no synchronization), you could potentially parallelize this to get some gains:

Parallel.ForEach(this.AllParameters, par =>
{
    foreach (Parameter subPar in par.WrappedSubParameters)
    {
        subPar.IsSelected = false;
    }
    par.IsSelected = false;
});

Note that I'm only parallelizing the outer loop (on purpose), as this should provide enough work items to adequately use all processing cores.

Another potential issue - if your IsSelected properties are bound to a control via Data binding, your UI will potentially be updating continually, which could explain the very slow update times. This would cause parallelization to have no real effect, as well, since the bottle neck would not be these loops, but rather the UI binding.

You may want to unbind/rebind the control, or suspend updates on the control until you're loop is completed.

Reed Copsey
  • 554,122
  • 78
  • 1,158
  • 1,373
  • I already tried this solution, But in my situation, it added not any difference. I am using x64 Computer with Windows 7 – Irfan Jul 17 '13 at 15:57
  • And now again, for the sake of experiment, I tried this code again, and somehow, my Program goes into not-responding state :( – Irfan Jul 17 '13 at 16:11
  • 1
    @Irfan I think the data binding is the issue. Are you using WPF, WinForms, Windows Store app, etc? – Reed Copsey Jul 17 '13 at 16:22
  • 1
    @Irfan: Likely it goes into not responding state because the parallel code is trying to modify a UI control from a thread pool thread. That's going to be a problem. Do this without the parallel stuff, and, as Lasse V. Karlsen suggested in a comment, suspend layout on your control before you start updating. – Jim Mischel Jul 17 '13 at 16:48
  • thanks ... so I will now put another question regarding suspending layout in my situation! – Irfan Jul 18 '13 at 08:16
1

The loop you have written is already the most efficient way to iterate. The only problem could be binding to User Interface elements (Grid, List, etc).

One workaround in MVVM is that unbind the databindings and after you are done itrating, bind them again.

Otherwise, do not set the actual properties, just set the fields, and after iteration you can notify the userinterface.