Click here to Skip to main content
14,970,309 members
Please Sign up or sign in to vote.
4.00/5 (1 vote)
See more:
kindly help me out so as to improve performance and dispose memory allocation in this code snippet
public static void GetBuildingCount(int marineCount, int[,] marinePositions)
       {
               int x = 0;
               int i = 0;
               int j = 0;
               bool processed = false;
               if (marineCount == (marinePositions.Length / 2))
               {
                   for (j = 1; j <= 8; j++)
                   {
                       processed = false;
                       for (i = 0; i <= marinePositions.Length / 2 - 1; i++)
                       {
                           if (j == marinePositions[i, 0] && processed == false)
                           {
                               x++;
                               output1 = output1 + 8;
                               processed = true;
                           }
                       }

                   }
                   for (j = 1; j <= 8; j++)
                   {
                       processed = false;
                       for (i = 0; i <= marinePositions.Length / 2 - 1; i++)
                       {
                           if (j == marinePositions[i, 1] && processed == false)
                           {
                               output1 = output1 + 8 - x;
                               processed = true;
                           }
                       }
                   }
               }

               System.GC.Collect();
               System.GC.WaitForPendingFinalizers();

           }
Posted
Updated 28-Jun-11 23:45pm
v2

If you want to improve performance, the first thing you should look at is to stop performing the same loop twice. Is there a reason you cannot combine the two together?

Also, don't force a garbage collection. In all the time I've been coding apps in .NET, I have never needed to force the garbage collector to process (and doing so causes a performance hit).
   
Comments
Tarun.K.S 29-Jun-11 6:01am
   
Excellent suggestion.
   
Agree, my 5.
--SA
Firstly, ditch your processed variable, and use break instead. This will cause that loop to terminate immediately, instead of running through after you have found what you want.
for (j = 1; j <= 8; j++)
{
    for (i = 0; i <= marinePositions.Length / 2 - 1; i++)
    {
        if (j == marinePositions[i, 0])
        {
            x++;
            output1 = output1 + 8;
            break;
        }
    }
}
This applies to both "j" loops.
Secondly, dump the Garbage Collection stuff - this routine does not create or dispose any data that the GC can handle - it is all local stack based variables.
   
Comments
   
Agree, my 5. Also, as Pete correctly pointed out, two loops should be too many :-).
--SA
If you're looking in Task Manager to see how muchmemory your app is using, stop it. TM isn't showing you how much memory your app is using, but is showing you how much memory is RESERVED by the .NET CLR running your app. You've got nothing to worry about. The CLR does a great job at managing memory and returning any unused memory back to Windows when necessary.
   
Comments
   
Agree, my 5. I also advices not to do manual collection, please see my answer.
--SA

This content, along with any associated source code and files, is licensed under The Code Project Open License (CPOL)




CodeProject, 20 Bay Street, 11th Floor Toronto, Ontario, Canada M5J 2N8 +1 (416) 849-8900