views:

1243

answers:

9

How do I improve the performance of the following situation?

I have an application that accepts multiple connections across a network. The application is written in C# and accepts Socket connections. Every five minutes, the application needs to perform a function call that updates the application and reports information back to the sockets. Truncated code follows

...
{
    new Thread(Loop).Start();
}

public Loop()
{
    ...
    while (true)
    {
        ...
        string line = user.Read();
        ...
    }
    ...
}

The above code is what is run when a Socket is connected to the server. The following code is what is run every five minutes.

...
{
    new Thread(TryTick).Start();
}

public void TryTick()
{
    while(true)
    {
        Tick();
        Thread.Sleep(new TimeSpan(0, 5, 0));
    }
}

Tick() does some File I/O operations as well as parsing a very limited (under 1MB) set of XML data. However, this code will tax my processor more than I had thought it would. Once a tick occurs, the system seems to grab an entire Core of my dual core development machine and doesn't let go. This seems to be something fairly simple, but perhaps I am doing it the easy way instead of the fast way.

This system is meant to handle up to 100 users, much more data and have a response time of under 1 second during load, so performance is an issue.

+1  A: 

Have you tried compiling in release mode to see if there is much difference in speed?

ck
This has improved the performance. But, with 1 user connected and a very minor set of data, it still operates at around 15-20% CPU. This may be enough to satisfy the requirements, though. Thanks for the info.
Derek Hammer
A: 

It looks as if your call to Tick() goes recursive.

I would also recommend that you add some code that allows you to insert gather better metrics about the function you are concerned about - e.g how long does it take to run the "Tick" function.

It can really help diagnose "oops!" problems such as these.

Andrew Grant
A: 

I think I would have to know what all you're doing in the Tick and why you're doing it in the Tick but whenever I am in a similar sort of situation I use EventWaitHandles

Allen
These appear to be C#'s implementation of semaphores. How are they applicable here?
Derek Hammer
A: 

You are calling your tick recursively. Btw, you can use a Timer which does exactly what you want in that case. Also note that it is probably giving stackoverflow exception after sometime - won't report it on your main excution as it is a thread (set up a global log for all unhandled exception to get info on that as well).

eglasius
See the updated code snippet. It was just a code copying error.
Derek Hammer
A: 

You could look at threading the Tick and file reading operations to see if the system can better distribute the load. IO is very expensive so having it run on a different thread can sometimes speed things up. Why don't you try the Parallel Extension for the .NET Framework to assist in that, they have some nice, easy API functions to assist in making loops operate in parallel.

Jeffrey Cameron
Looks interesting. If I need to improve the performance further, then I will most definitely look into it.
Derek Hammer
+4  A: 

You really need to profile this code before deciding what to do . . . if you don't have access to a profiler, add simple instrumentation to determine where the bottleneck is. Start with timing Tick() to verify it is the source of the problems (as opposed to somewhere else in your program), and then repeat for sub-sections of Tick() as needed.

You really need to profile and measure here . . . making presumptions about where your perf problem lies could lead you to waste a lot of time.

Michael
A: 

I noticed in the public Loop(), in the while(true) loop you haven't included a Thread.Sleep(0). If you haven't, you should consider adding it. Thread.Sleep(0) will tell the processor to use the remaining of the time allocated to that thread for other threads/processes that may require it. Without such a call you're essentially hanging onto processor time for no good reason.


  public Loop()
  {
      ...
      while (true)
      {
          ...
          string line = user.Read();
          ...
          Thread.Sleep(0);
      }
      ...
  }

This may no longer be an issue for you, and my suggestion may not be applicable to your case but since you haven't disclosed the full source it's hard to know exactly the root of the problem. It's just something to think about.

+2  A: 

You are using a while(true). Most .NET socket stuff should implement event handlers of some form. You can make custom generic Event Handlers so that way the listener just really does nothing else other than listen for connections and forward on to the event. Some examples of socket recieve handlers are located http://www.c-sharpcorner.com/Forums/ShowMessages.aspx?ThreadID=37717

When you put everything inside a while loop and use Thread.Sleep() you are still using the processor while the Thread is "sleeping".

Also make sure that you are doing a Thread.Join() when you are done using the thread. Otherwise you will end up with many many threads that are waiting for work and doing nothing other than taking up excess memory and processor.

Typically if you have a while(true) loop in your code and a Thread.Sleep() you should ask yourself if you are doing something wrong. In most cases you are, and the best point of refactoring that is to figure out the way to handle requests without using resources.

jwendl
All other answers will probably go in the direction of sleeping the thread again.. This one is the correct one I think. I feel that in general a while(true) should be avoided...
borisCallens