Solved

C# Improve Threading Performance

Posted on 2011-09-12
8
398 Views
Last Modified: 2012-05-12
Hi

I have the following code as an example. I need to improve this code example that is very similar to the real code. Please if someone can help with some tips about improve this example. Thanks

class Program
    {
        static void Main(string[] args)
        {
            Game myGame = new Game();
            myGame.RunIt();
        }
    }


    class Game
    {
        List<Player> listPlayers = new List<Player>();
        Thread WrkThread = null;
        public void RunIt()
        {
            // set up the players
            listPlayers.Add(new Player("Tom", 100.0));
            listPlayers.Add(new Player("Jerry", 100.0));
            listPlayers.Add(new Player("Homer", 100.0));
            listPlayers.Add(new Player("Lisa", 200.0));

            // start a new thread that randomly redistribute their money
            WrkThread = new Thread(new ThreadStart(ThreadHandler));
            WrkThread.Start();

            // check periodically to remove losers
            while (true)
            {
                Thread.Sleep(1000);
                foreach (var x in listPlayers)
                {
                    if (x.money < 0.0)
                    {
                        Console.WriteLine("{0} has lost all money. Remove from table.", x.name);
                        listPlayers.Remove(x);
                    }
                }
            }
        }

        protected virtual void ThreadHandler()
        {
            Random rand = new Random();
            DateTime nextHandTime = DateTime.Now;
            while (true)
            {
                if (DateTime.Now < nextHandTime) continue;
                nextHandTime = DateTime.Now.AddSeconds(5);
                int winner = rand.Next(listPlayers.Count);
                double pot = 0;
                for (int i = 0; i < 4; i++)
                {
                    if (i != winner)
                    {
                        listPlayers[i].money -= 5.0;
                        pot += 5.0;
                    }
                }
                listPlayers[winner].money += pot;
            }
        }
    }


    class Player
    {
        public string name;
        public double money;
        public Player(string name1, double money1)
        {
            name = name1;
            money = money1;
        }
    }

Open in new window

0
Comment
Question by:JoseHidalgo
[X]
Welcome to Experts Exchange

Add your voice to the tech community where 5M+ people just like you are talking about what matters.

  • Help others & share knowledge
  • Earn cash & points
  • Learn & ask questions
  • 4
  • 3
8 Comments
 
LVL 75

Expert Comment

by:käµfm³d 👽
ID: 36526913
I know this is going to sound silly, and I'm not trying to be rude, but have you done much threading?
0
 

Author Comment

by:JoseHidalgo
ID: 36526931
Actually is a small test. I submited this code but the lead is telling me that this code still have many problems that I need to tell him what are the problems.

Thanks
0
 

Author Comment

by:JoseHidalgo
ID: 36526944
For example

When the collection remove a player it will throw an error:

Collection was modified; enumeration operation may not execute.

0
Major Incident Management Communications

Major incidents and IT service outages cost companies millions. Often the solution to minimizing damage is automated communication. Find out more in our Major Incident Management Communications infographic.

 
LVL 75

Assisted Solution

by:käµfm³d 👽
käµfm³d   👽 earned 250 total points
ID: 36526965
The big thing with threading is the sharing of resources and/or objects. You have a shared object in listPlayers. Any time you have a shared object or resource, you can encounter instances of deadlocking (two threads each waiting for the other to release some resource) or race conditions (two threads trying to acquire some resource but who gets the resource is dependent on the results of the operations in the other thread). In your case, you have a race condition. Take a look at lines 36 and 50. Let's say the thread running line 50 is currently running. Line 50 executes. Then a context switch* occurs. Now you are running the thread that is on line 36. Line 36 is executed. Then another context switch occurs. Let's say you get to line 54. The value of winner is now 4 (because you initally loaded 4 objects into the list). However, line 36 has already executed on the other thread. How many items are actually in your list?


* A context switch is essentially when the CPU saves the current thread, loads a waiting thread, and begins executing it.
0
 
LVL 75

Expert Comment

by:käµfm³d 👽
ID: 36526970
The value of winner is now 4
I didn't factor in the use of the rand.Next call, but the underlying concept I am discussing still holds.
0
 
LVL 75

Expert Comment

by:käµfm³d 👽
ID: 36526991
For example

When the collection remove a player it will throw an error:

Collection was modified; enumeration operation may not execute.
That error is not due to threading; rather it is because you are calling the Remove method inside of a foreach. You cannot remove items from a collection while inside of a foreach that is iterating over that same collection. To remove items in this manner use a for loop and do reverse iteration (i.e. length - 1 => 0).
0
 
LVL 40

Accepted Solution

by:
Jacques Bourgeois (James Burger) earned 250 total points
ID: 36527032
This might not make a big difference if you have only 4 players, but if the real application has more than that, calling AddRange once will all the players might improve the performance over adding them one by one.

-----

Then, a big question: why use a separate thread? This is a current misconception that threading will improve everything. 98% of the times i hear "Would multi-threading improve that routine?", the answer is No!

From what we have, your application does nothing but loop while the thread is running. Threading is usefull when many operations needs to run simultaneously. Starting a Thread and simply waiting for it to change something is overkill, you are making things more complex than could be.

Then, a loop that does almost nothing takes up CPU ticks anyway. That is one the best ways to lose performance, not enhance it.

We do not see the big picture, because "code example that is very similar to the real code" is not the same as the real code, and that can make a big difference. But from what I see there, a simple Timer would do the job more efficiently because you would not lose the CPU in the loop. Timers are usually the most effective way regularly check for a situation.

-----

For Collection was modified, this is normal. You cannot remove an element from a collection while you are looping through it with a foreach loop, because once the element is gone, so is the pointer to the next element. This is because of the way collections are stored and handled in memory. The problem can sometimes be solved by using a regular loop, the one with a counter, starting from the end going down (decrement the index instead of incrementing it). But it does not work will all collections, once again because of the way they are stored ans handled in memory.

Have a property in the Player class that defines if a player is active or not, and simply set that to False. The logic of the application would then be adapted to that property.
0
 

Author Closing Comment

by:JoseHidalgo
ID: 36527125
Thanks for the responses. Help me a lot
0

Featured Post

Technology Partners: We Want Your Opinion!

We value your feedback.

Take our survey and automatically be enter to win anyone of the following:
Yeti Cooler, Amazon eGift Card, and Movie eGift Card!

Question has a verified solution.

If you are experiencing a similar issue, please ask a related question

A long time ago (May 2011), I have written an article showing you how to create a DLL using Visual Studio 2005 to be hosted in SQL Server 2005. That was valid at that time and it is still valid if you are still using these versions. You can still re…
When there is a disconnect between the intentions of their creator and the recipient, when algorithms go awry, they can have disastrous consequences.
I've attached the XLSM Excel spreadsheet I used in the video and also text files containing the macros used below. https://filedb.experts-exchange.com/incoming/2017/03_w12/1151775/Permutations.txt https://filedb.experts-exchange.com/incoming/201…
If you're a developer or IT admin, you’re probably tasked with managing multiple websites, servers, applications, and levels of security on a daily basis. While this can be extremely time consuming, it can also be frustrating when systems aren't wor…

726 members asked questions and received personalized solutions in the past 7 days.

Join the community of 500,000 technology professionals and ask your questions.

Join & Ask a Question