none
Deleting data from List<T> after 4 foreach iteration causes system freeze when dealing with many data RRS feed

  • Question

  • see the code

    foreach (var itemBroker in lstBrokers)
    {
    	foreach (var itemSectionLineItem in lstSectionLineItem)
    	{
    		foreach (var itemPeriod in lstPeriod)
    		{
    
    
    			//=============If multiple item found then item will be deleted and inserted
    			int iCount = lstComments.Where(b => b.BrokerFor.ToString().Trim().ToLower() == itemBroker.ToString().Trim().ToLower()
    													&& b.SectionFor.ToString().Trim().ToLower() == itemSectionLineItem._Section.ToString().Trim().ToLower()
    													&& b.LiFor.ToString().Trim().ToLower() == itemSectionLineItem._LineItem.ToString().Trim().ToLower()
    				//  && b.PeriodCollection.ToString().Trim().ToLower().Replace("A", "").Replace("E", "") == itemPeriod.ToString().Trim().ToLower().Replace("A", "").Replace("E", "")
    													  && b.PeriodCollection.ToString().Trim().Replace("A", "").Replace("E", "").Contains(itemPeriod.ToString().Trim().Replace("A", "").Replace("E", ""))
    													).Count();
    
    			for (int i = 0; i < iCount; i++)
    			{
    				int iPosition = lstComments.FindIndex(b => b.BrokerFor.ToString().Trim().ToLower() == itemBroker.ToString().Trim().ToLower()
    														&& b.SectionFor.ToString().Trim().ToLower() == itemSectionLineItem._Section.ToString().Trim().ToLower()
    														&& b.LiFor.ToString().Trim().ToLower() == itemSectionLineItem._LineItem.ToString().Trim().ToLower()
    					//  && b.PeriodCollection.ToString().Trim().ToLower().Replace("A", "").Replace("E", "") == itemPeriod.ToString().Trim().ToLower().Replace("A", "").Replace("E", "")
    														  && b.PeriodCollection.ToString().Trim().Replace("A", "").Replace("E", "").Contains(itemPeriod.ToString().Trim().Replace("A", "").Replace("E", ""))
    														);
    				if (iPosition >= 0)
    				{
    					lstComments.RemoveAt(iPosition);
    				}
    			}
    		}
    	}
    }
    my lstComments has many data say 50,000 data. i am iterating in 3 foreach loop. finding data in list. if found then deleting data from list lstComments. the problem is my application is getting freeze
    when dealing with many data but when dealing with small data then facing no issue. so guide me how to restructure the above code which would be faster when dealing with many data
    and also system will not freeze. please give me rectified code which will be performant.

    Thanks


    • Edited by Sudip_inn Tuesday, November 5, 2019 10:12 AM
    Tuesday, November 5, 2019 10:12 AM

Answers

  • Hi Sudip,
     
    What is probably a better idea, is to create a list of the Positions, sort it, and then iterate backwards through the lstComments and RemoveAt() that way.
     
    List<int> lstPosition = new List<int>();
     
    for (int i = 0; i < iCount; i++)
    {
        int iPosition = lstComments.FindIndex(b => b.BrokerFor.ToString().Trim().ToLower() == itemBroker.ToString().Trim().ToLower()
                                                && b.SectionFor.ToString().Trim().ToLower() == itemSectionLineItem._Section.ToString().Trim().ToLower()
                                                && b.LiFor.ToString().Trim().ToLower() == itemSectionLineItem._LineItem.ToString().Trim().ToLower()
            //  && b.PeriodCollection.ToString().Trim().ToLower().Replace("A", "").Replace("E", "") == itemPeriod.ToString().Trim().ToLower().Replace("A", "").Replace("E", "")
                                                    && b.PeriodCollection.ToString().Trim().Replace("A", "").Replace("E", "").Contains(itemPeriod.ToString().Trim().Replace("A", "").Replace("E", ""))
                                                );
        if (iPosition >= 0)
        {
            lstPosition.Add(iPosition);
        }
    }
     
    lstPosition.Sort();
     
    // remove in reverse order
    for (int j = lstPosition.Count - 1; j >= 0; j--)
    {
        lstComments.RemoveAt(j);
    }



    ~~Bonnie DeWitt [C# MVP]

    http://geek-goddess-bonnie.blogspot.com

    • Marked as answer by Sudip_inn Saturday, November 9, 2019 5:33 PM
    Wednesday, November 6, 2019 6:10 AM
    Moderator
  • Hi Sudip,

    Thank you for posting here.

    According to your description, your program will freeze when it runs.

    I think the problem is mainly on a multi-level nested loop.

    I made a simple example by simulating your code.

    Here is the code:

    class Program { static void Main(string[] args) { List<Comments> lstComments = new List<Comments>(); for (int i = 0; i < 100; i++) { lstComments.Add(new Comments(1, 1, 1)); } for (int i = 0; i < 100; i++) { lstComments.Add(new Comments(2, 2, 2)); } for (int i = 0; i < 100; i++) { lstComments.Add(new Comments(3, 3, 3)); } List<int> list1 = GetList(); List<int> list2 = GetList(); List<int> list3 = GetList(); int number = 0; foreach (var num1 in list1) { foreach (var num2 in list2) { foreach (var num3 in list3) { int count = lstComments.Where(b => b.Num1 == num1 && b.Num2 == num2 && b.Num3 == num3).Count(); number++; } } } Console.WriteLine(number); Console.WriteLine("Press any key to continue..."); Console.ReadKey(); } public static List<int> GetList() { List<int> list = new List<int>(); for (int i = 0; i < 100; i++) { list.Add(i); } return list; } } class Comments { public int Num1 { get; set; } public int Num2 { get; set; } public int Num3 { get; set; } public Comments(int n1, int n2,int n3) { this.Num1 = n1; this.Num2 = n2; this.Num3 = n3; } }


    foreach (var num in list1)
    {
       lstComments.Where(b => b.Num1 == num);
       number++;
    }
    foreach (var num in list2)
    {
        lstComments.Where(b => b.Num2 == num);
        number++;
    }
    foreach (var num in list3)
    {
        lstComments.Where(b => b.Num3 == num);
        number++;
    }

    Because the innermost loop will executed 1,000,000 times, this program will run for a while to display the results.

    Then I modified the code.

    I used three single-level loops instead of nested loops.

    The program runs quickly, because the loop only executes 300 times.

    For your code, you can also split the nested loop and filter the lstComments multiple times.

    The code may look a lot more, but it will greatly improve the efficiency of your program.

    Hope my solution could be helpful.

    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.





    Wednesday, November 6, 2019 9:54 AM

All replies

  • every time you RemoveAt, a new array has to be allocated and the data moved
    so you could utilize a two-staged approach: in the first iteration only set the items you want to remove to null instead of RemoveAt,
    and then in the second stage only take the non-null values into a new list:
    lstComments = lstComments.Where(l => l != null).ToList();

    btw.: you are creating a lot of new string instances with your constant ".ToString().Trim().ToLower()".
    And your Replacings
    Perhaps you should Trim the data once at the beginning (why is this even necessary?)
    then StringComparers.OrdinalIgnoreCase can avoid the ".ToLower()" calls 

    And sorry, but your code looks convoluted.
    Tuesday, November 5, 2019 5:59 PM

  • foreach (var itemBroker in lstBrokers)
    {
    	foreach (var itemSectionLineItem in lstSectionLineItem)
    	{
    		foreach (var itemPeriod in lstPeriod)
    		{
    
    
    			//=============If multiple item found then item will be deleted and inserted
    			int iCount = lstComments.Where(b => b.BrokerFor.ToString().Trim().ToLower() == itemBroker.ToString().Trim().ToLower()
    													&& b.SectionFor.ToString().Trim().ToLower() == itemSectionLineItem._Section.ToString().Trim().ToLower()
    													&& b.LiFor.ToString().Trim().ToLower() == itemSectionLineItem._LineItem.ToString().Trim().ToLower()
    				//  && b.PeriodCollection.ToString().Trim().ToLower().Replace("A", "").Replace("E", "") == itemPeriod.ToString().Trim().ToLower().Replace("A", "").Replace("E", "")
    													  && b.PeriodCollection.ToString().Trim().Replace("A", "").Replace("E", "").Contains(itemPeriod.ToString().Trim().Replace("A", "").Replace("E", ""))
    													).Count();
    
    			for (int i = 0; i < iCount; i++)
    			{
    				int iPosition = lstComments.FindIndex(b => b.BrokerFor.ToString().Trim().ToLower() == itemBroker.ToString().Trim().ToLower()
    														&& b.SectionFor.ToString().Trim().ToLower() == itemSectionLineItem._Section.ToString().Trim().ToLower()
    														&& b.LiFor.ToString().Trim().ToLower() == itemSectionLineItem._LineItem.ToString().Trim().ToLower()
    					//  && b.PeriodCollection.ToString().Trim().ToLower().Replace("A", "").Replace("E", "") == itemPeriod.ToString().Trim().ToLower().Replace("A", "").Replace("E", "")
    														  && b.PeriodCollection.ToString().Trim().Replace("A", "").Replace("E", "").Contains(itemPeriod.ToString().Trim().Replace("A", "").Replace("E", ""))
    														);
    				if (iPosition >= 0)
    				{
    					lstComments.RemoveAt(iPosition);
    				}
    			}
    		}
    	}
    }

    I haven't analyzed your code at all, but I feel uneasy about the way you are
    removing items from the list. 

    You have a for loop which tests the variable iCount. That variable appears to 
    have been set earlier in the code from lstComments.Count, but Count will change
    whenever you do a lstComments.RemoveAt(). So the contents of iCount may no 
    longer be valid after RemoveAt().

    - Wayne

    Wednesday, November 6, 2019 1:56 AM
  • Hi Sudip,
     
    What is probably a better idea, is to create a list of the Positions, sort it, and then iterate backwards through the lstComments and RemoveAt() that way.
     
    List<int> lstPosition = new List<int>();
     
    for (int i = 0; i < iCount; i++)
    {
        int iPosition = lstComments.FindIndex(b => b.BrokerFor.ToString().Trim().ToLower() == itemBroker.ToString().Trim().ToLower()
                                                && b.SectionFor.ToString().Trim().ToLower() == itemSectionLineItem._Section.ToString().Trim().ToLower()
                                                && b.LiFor.ToString().Trim().ToLower() == itemSectionLineItem._LineItem.ToString().Trim().ToLower()
            //  && b.PeriodCollection.ToString().Trim().ToLower().Replace("A", "").Replace("E", "") == itemPeriod.ToString().Trim().ToLower().Replace("A", "").Replace("E", "")
                                                    && b.PeriodCollection.ToString().Trim().Replace("A", "").Replace("E", "").Contains(itemPeriod.ToString().Trim().Replace("A", "").Replace("E", ""))
                                                );
        if (iPosition >= 0)
        {
            lstPosition.Add(iPosition);
        }
    }
     
    lstPosition.Sort();
     
    // remove in reverse order
    for (int j = lstPosition.Count - 1; j >= 0; j--)
    {
        lstComments.RemoveAt(j);
    }



    ~~Bonnie DeWitt [C# MVP]

    http://geek-goddess-bonnie.blogspot.com

    • Marked as answer by Sudip_inn Saturday, November 9, 2019 5:33 PM
    Wednesday, November 6, 2019 6:10 AM
    Moderator
  • Hi Sudip,

    Thank you for posting here.

    According to your description, your program will freeze when it runs.

    I think the problem is mainly on a multi-level nested loop.

    I made a simple example by simulating your code.

    Here is the code:

    class Program { static void Main(string[] args) { List<Comments> lstComments = new List<Comments>(); for (int i = 0; i < 100; i++) { lstComments.Add(new Comments(1, 1, 1)); } for (int i = 0; i < 100; i++) { lstComments.Add(new Comments(2, 2, 2)); } for (int i = 0; i < 100; i++) { lstComments.Add(new Comments(3, 3, 3)); } List<int> list1 = GetList(); List<int> list2 = GetList(); List<int> list3 = GetList(); int number = 0; foreach (var num1 in list1) { foreach (var num2 in list2) { foreach (var num3 in list3) { int count = lstComments.Where(b => b.Num1 == num1 && b.Num2 == num2 && b.Num3 == num3).Count(); number++; } } } Console.WriteLine(number); Console.WriteLine("Press any key to continue..."); Console.ReadKey(); } public static List<int> GetList() { List<int> list = new List<int>(); for (int i = 0; i < 100; i++) { list.Add(i); } return list; } } class Comments { public int Num1 { get; set; } public int Num2 { get; set; } public int Num3 { get; set; } public Comments(int n1, int n2,int n3) { this.Num1 = n1; this.Num2 = n2; this.Num3 = n3; } }


    foreach (var num in list1)
    {
       lstComments.Where(b => b.Num1 == num);
       number++;
    }
    foreach (var num in list2)
    {
        lstComments.Where(b => b.Num2 == num);
        number++;
    }
    foreach (var num in list3)
    {
        lstComments.Where(b => b.Num3 == num);
        number++;
    }

    Because the innermost loop will executed 1,000,000 times, this program will run for a while to display the results.

    Then I modified the code.

    I used three single-level loops instead of nested loops.

    The program runs quickly, because the loop only executes 300 times.

    For your code, you can also split the nested loop and filter the lstComments multiple times.

    The code may look a lot more, but it will greatly improve the efficiency of your program.

    Hope my solution could be helpful.

    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.





    Wednesday, November 6, 2019 9:54 AM