HiveBrain v1.2.0
Get Started
← Back to all entries
snippetcsharpMinor

How to optimize these nested loops for better performance?

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
theseloopsbetternestedoptimizeforhowperformance

Problem

I need to optimize this code so it can execute faster, even if that means using more memory:

for (int t = 0; t < Clientes[0].ChildNodes.Count; t++)
{
    for (int i = 0; i < Clientes[0].ChildNodes.Item(t).ChildNodes.Count; i++)
    {
        if (Clientes[0].ChildNodes.Item(t).ChildNodes.Item(i).Name.Contains("Service"))
        {
            int a = 0;
            int.TryParse(Clientes[0].ChildNodes.Item(t).ChildNodes.Item(i).Name.ToUpper().Replace("SERVICE", ""), out a);
            if (a.Equals(Type))
            {
                Queues[t].Send(QueueMessage);
            }
        }
    }
}


I have decided to avoid accessing Clientes[0] and its ChildNodes.Item(t) over and over in each loop by keeping them in new variables like this:

XmlNodeList clientZeroChildNodes ;
int clientZeroChildCount ;

XmlNodeList clientZeroItemTchildNodes ;
int clientZeroItemTchildCount;
string clientZeroItemTchildIname;

clientZeroChildNodes = Clientes[0].ChildNodes;
clientZeroChildCount = clientZeroChildNodes.Count;

for (int t = 0 ; t < clientZeroChildCount ; t++)
{
    clientZeroItemTchildNodes = clientZeroChildNodes.Item(t).ChildNodes;
    clientZeroItemTchildCount = clientZeroItemTchildNodes.Count;

    for (int i = 0; i < clientZeroItemTchildCount; i++)
    {
        clientZeroItemTchildIname = clientZeroItemTchildNodes.Item(i).Name;

        if (clientZeroItemTchildIname.Contains("Service"))
        {
            int a = 0;

            int.TryParse(clientZeroItemTchildIname.ToUpper().Replace("SERVICE", ""), out a);

            if (a.Equals(Type))
            {
                try
                {
                    Queues[t].Send(QueueMessage);
                }
                catch (MessageQueueException mqe)
                {
                    //log error about mqe
                }
            }
        }
    }

}


As you can see, the second for loop only takes certain items of the ChildNodes based on its name. I have tried to use LINQ so that

Solution

I would highly suggest profiling your code to see where the bottleneck lies.

That said, one thing to note (aside from what has already been mentioned) is that XmlNodeList.Item(int) is typically \$O(n)\$ *. As a result, your loops are \$O(n^2)\$.

If your elements have a large number of children (100+), you may want to switch to foreach loops, which will be \$O(n)\$:

foreach (XmlNode node in foo.ChildNodes)
{
   // Do stuff.
}


* The IL for System.Xml.dll reveals that XmlNode.ChildNodes returns type XmlChildNodes, and XmlChildNodes.Item(int) is \$O(n)\$.

Code Snippets

foreach (XmlNode node in foo.ChildNodes)
{
   // Do stuff.
}

Context

StackExchange Code Review Q#30881, answer score: 7

Revisions (0)

No revisions yet.