locked
Performance Issue in Nested foreach Loop for Filtering Collection RRS feed

  • Question

  • Performance Issue in Nested foreach Loop for Filtering Collection..

    I tried 2 Ways... 

     1.

    	
    				 var routineArray = new ObservableCollection<IRoutine>();
    				 IOrderedEnumerable<string> strNames = null;
                     var enumerable = getRoutines as IRoutine[] ?? getRoutines;
    				  var routines = enumerable.ToList();
    
    
    			foreach (var st in strNames)
                {
                    IRoutine first = null;
                    foreach (var x in routines)
                    {
                        if (x.Name == st)
                        {
                            first = x;
                            break;
                        }
                    }
    
                    routineArray.Add(first);
                }

    2.

    foreach (var st in strNames)
                      routineArray.Add(routines.FirstOrDefault(x => x.Name == st));

    Both are causing performance Issue while filtering and updating UI UWP.

    Sunday, August 2, 2020 4:23 PM

All replies

  • You could try a LINQ join. Something similar to the following:

    routineArray.AddRange(
       from st in strNames
       join x in routines on st equals x.Name
       select x
    );

    The internal implementation of the join uses a hash table, so it should perform better than nested loops.

    You can also experiment with "Contains", although the internal implementation of this method uses a loop:

    routineArray.AddRange(routines.Where(x => strNames.Contains(x.name));

    • Proposed as answer by Naomi N Monday, August 3, 2020 4:12 PM
    Sunday, August 2, 2020 5:15 PM
  • Tried but no Difference...
    Monday, August 3, 2020 4:42 AM
  • You could try a LINQ join. Something similar to the following:

    routineArray.AddRange(
       from st in strNames
       join x in routines on st equals x.Name
       select x
    );

    The internal implementation of the join uses a hash table, so it should perform better than nested loops.

    You can also experiment with "Contains", although the internal implementation of this method uses a loop:

    routineArray.AddRange(routines.Where(x => strNames.Contains(x.name));

    Tried but no Difference
    Monday, August 3, 2020 4:44 AM
  • How large strNames and routines are?

    Maybe consider a different approach:

       var names = strNames.ToList();

       var routines = enumerable;

       foreach( var routine in routines)

       {

          if( names.BinarySearch( routines.Name ) >= 0)

          {

             routineArray.Add(routine);

          }

       }


    • Edited by Viorel_MVP Monday, August 3, 2020 5:46 AM
    Monday, August 3, 2020 5:43 AM
  • Little Better, but results are different, 

    foreach (var st in strNames)
                      routineArray.Add(routines.FirstOrDefault(x => x.Name == st));
    Monday, August 3, 2020 7:03 AM
  • I doubt the above code is actually the issue but rather likely it is the strNames. Of course you posted a subset of the overall code so we are making assumptions that might or might not be true.

    1. strNames is null here so it would never work. But it is typed as IOrderedEnumerable<string>. If that ordering is happening against a large list of strings it could take a while. Consider calling ToArray on it before the loop and profile that call to see if it takes a while.

    2. routineArray is an expensive ObservableCollection which you probably don't need at this point. Use a regular List<T> and convert it to ObservableCollection later after you're done.

    3. `enumerable` is redundant here. You fetch the value of `getRoutines` and cast to an array. If that fails then you use `getRoutines` anyway. So why bother with the check at all. Just treat it as an `IEnumerable<IRoutine>`. Also, converting to a list on the next line is expensive if there are a lot of them so unless it is really needed then don't bother. 

    4. The inner foreach can be done with FirstOrDefault and it will be less code (as you've already done). But you are also making the assumption that a routine can be found. If it can't then you're putting nulls into your array which doesn't seem right to me.

    Like I said earlier I suspect the issue is with `strNames` but you can wrap each call to the collections with a stopwatch (or using the later versions of VS that show execution time) and figure out exactly which line is slow. Break up your multiple expressions into separate expressions (1 per line) to make this easier to diagnose. Then you can put it all back together after you've identified the perf issue.



    Michael Taylor http://www.michaeltaylorp3.net

    • Proposed as answer by Naomi N Monday, August 3, 2020 4:14 PM
    Monday, August 3, 2020 2:33 PM
  • Hi ID GO,

    Has your issue been resolved?

    If so, please click on the "Mark as answer" option of the reply that solved your question, so that it will help other members to find the solution quickly if they face a similar issue.

    Best Regards,

    Timon


    MSDN Community Support
    Please remember to click "Mark as Answer" the responses that resolved your issue, and to click "Unmark as Answer" if not. This can be beneficial to other community members reading this thread. If you have any compliments or complaints to MSDN Support, feel free to contact MSDNFSF@microsoft.com.

    Friday, August 21, 2020 6:08 AM