Loop inside a loop performance

Question

The only reason I'm posting this is because I actually believe that my code perfomance is being affected by this code block where I use a foreach loop inside another one.

I was wondering if anyone would suggest anything different that could help the performance or perhaps point out some other flaws in the code.

    public override void DisplayScore()
    {
        byte MessageLocation = 0;
        foreach (var kvp in PlayerScores.OrderByDescending((s => s.Value)))
        {
            if (MessageLocation == 5)
                break;

            if (MessageLocation == PlayerScores.Count)
                break;

            foreach (var player in PlayerList.Values)
            {
                SendMessage(MessageLocation, "My text");
            }
            Score++;
        }
    }

As you can see, it's just displaying the top 5 scores (in different locations) from a dictionary in a descending order and sending them to a list of players from another dictionary.

what is MessageLocation and where you are updating its value? — sujith karivelil
– sujith karivelil, Commented Jun 21, 2016 at 4:30
Your code looks bad, but it definitely does not generate any performance problems. Such simple constructions like foreach, break etc. will not cause performance problems until you have millions of records. Try to profile it, or at least try to debug it. Most probably, the problem is somewhere in SendMessage. — Yeldar Kurmangaliyev
– Yeldar Kurmangaliyev, Commented Jun 21, 2016 at 4:30
Process it in parallel, with for example Parallel.ForEach (easiest method). This way it will be as fast as you will get it for 2 simple loops i guess. — Viezevingertjes
– Viezevingertjes, Commented Jun 21, 2016 at 4:50
We can't really tell what's wrong without knowing how long things are taking, how large your collections are, or what SendMessage does. Please post a minimal reproducible example which demonstrates the problem. — Jon Skeet
– Jon Skeet, Commented Jun 21, 2016 at 6:13

scher · Accepted Answer · 2016-06-21 08:28:15Z

1

I don't think that the double loop is the problem. I suggest to check the LINQ query PlayerScores.OrderByDescending((s => s.Value)). Depending on the numbers of scores this might take its time to order, espesially if the values come from a dictionary. The internal structure of a dictinary makes it expensive to enumerate through keys and values.

You can test it with the following code (slightly improved) and Visual Studio 2015, where it can be seen, how long single execution steps take:

public override void DisplayScore()
{

    var scores = PlayerScores.OrderByDescending(s => s.Value).Take(5).ToArray();

    foreach (var kvp in scores)
    {
        foreach (var player in PlayerList.Values)
        {
            SendMessage(MessageLocation, "My text");
        }
    }
}

edited Jun 21, 2016 at 8:28

answered Jun 21, 2016 at 5:40

scher

1,9532 gold badges21 silver badges43 bronze badges

Sign up to request clarification or add additional context in comments.

2 Comments

Jon Skeet Over a year ago

I don't believe it's actually expensive to iterate over all the entries in a dictionary - it's just that it's not as quick as looking up by keys. Also note that the assignment to scores in your code will be very quick due to the lazy nature of LINQ queries - it's not actually doing any of the sorting at that point. If you wanted to actually split the LINQ part from the rest, you'd need to materialize the query.

scher Over a year ago

Yes your right with the lazyness of LINQ. I edited my answer.

Xela · Accepted Answer · 2016-06-21 04:31:07Z

0

Instead of nested for loops you may gain some performance by adding the scores to a list and then sending them to the players eg:

//List containing player and score

foreach (var kvp in PlayerScores.OrderByDescending((s => s.Value)))
{
         //Add scores to list       
}

foreach (var player in PlayerList.Values)
{
         //Send scores to players
}

answered Jun 21, 2016 at 4:31

Xela

2,4221 gold badge19 silver badges33 bronze badges

Collectives™ on Stack Overflow

Loop inside a loop performance

2 Answers 2

2 Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

2 Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related