HTTP Performance Test in C#

Question

I wrote a script to test how long it takes to make requests against a url. It takes a list of urls, an array of concurrent requests, and a number of times to attempt the test. How could I improve on this?

//https://www.nuget.org/packages/RestSharp
string baseUrl = "http://localhost";
void Main()
{
 var urls = new string[] { 
 "/some/end/point",
 };
 var concurrentRequests = new int[]{ 1, 5, 25, 50, 100, 500, 1000};
 var numberOfTurns = 10;
 RunTests(urls, concurrentRequests, numberOfTurns);
}
private void RunTests(string[] urls, int[] concurrentRequests, int numberOfTimes = 1)
{
 for(var i = 0; i < numberOfTimes; i++)
 {
 RunFor(urls, concurrentRequests);
 }
}
private void RunFor(string[] urls, int[] concurrentRequests)
{
 concurrentRequests.ToList().ForEach( e => 
 {
 RunTest(urls, e);
 }); 
}
private void RunTest(string[] sites, int iterations, string description = "")
{
 var action = GetAction();
 var watch = new Stopwatch(); 
 // Construct started tasks
 Task<bool>[] tasks = new Task<bool>[sites.Count()];
 watch.Start();
 for(int j = 0; j < iterations; j++)
 {
 for (int i = 0; i < sites.Count(); i++)
 {
 tasks[i] = Task<bool>.Factory.StartNew(action, sites[i]);
 }
 }
 try
 {
 Task.WaitAll(tasks);
 }
 catch (AggregateException e)
 {
 Console.WriteLine("\nThe following exceptions have been thrown by WaitAll()");
 for (int j = 0; j < e.InnerExceptions.Count; j++)
 {
 Console.WriteLine("\n-------------------------------------------------\n{0}", e.InnerExceptions[j].ToString());
 }
 }
 finally
 {
 watch.Stop();
 Console.WriteLine("\"{0}|{1}|{2}\", ",sites.Count(), iterations, watch.Elapsed.TotalSeconds);
 }
}
private Func<object, bool> GetAction()
{
 baseUrl = baseUrl.Trim('/');
 return (object obj) =>
 {
 var str = (string)obj;
 var client = new RestClient(baseUrl);
 client.Authenticator = new NtlmAuthenticator();
 var request = new RestRequest(str, Method.GET);
 request.AddHeader("Accept", "text/html");
 var response = client.Execute(request); 
 return (response != null);
 };
}

Answer 1

The default number of concurrent requests to the same server (ServicePointManager.DefaultConnectionLimit) is only 2. So I believe most of your tests are going to be limited by that and not by the server you're trying to test.

You should change the value to something larger (probably concurrentRequests.Max()).

---

concurrentRequests.ToList().ForEach( e => 
{
 RunTest(urls, e);
});

I don't see any reason to use List.ForEach() here (or pretty much anywhere else). Normal foreach is simpler and more efficient (no need to copy the array to a List):

foreach (int requests in concurrentRequests)
{
 RunTest(urls, requests);
}

I have also change the name of the iteration variable to request to make the code somewhat more readable.

---

for(int j = 0; j < iterations; j++)
for (int i = 0; i < sites.Count(); i++)

You should be consistent regarding whether you put a space after the for keyword. The common way is to put a space there.

---

tasks[i] = Task<bool>.Factory.StartNew(action, sites[i]);

This can be simplified to:

tasks[i] = Task.Factory.StartNew(action, sites[i]);

Also, unless few extra allocations are important for you, I would use a lambda here to make the code more type-safe (no more casts in GetAction) and clearer (your GetAction returns a lambda, my TestUrl is a normal method):

var url = urls[i];
tasks[i] = Task.Factory.StartNew(() => TestUrl(url));

Few notes about this code:

• I have changed sites to urls, it make it consistent between methods and also because I believe that's more correct term.
• The url variable is important here, Task.Factory.StartNew(() => TestUrl(urls[i])) wouldn't work correctly, because the i variable is shared among iterations of the loop, while url isn't.

---

for(int j = 0; j < iterations; j++)
{
 for (int i = 0; i < sites.Count(); i++)
 {
 tasks[i] = Task<bool>.Factory.StartNew(action, sites[i]);
 }
}

If iterations is greater than 1, you're going to overwrite old Tasks with new ones, which means you're not going to wait for the old tasks to complete. That's not okay.

---

Console.WriteLine("\nThe following exceptions have been thrown by WaitAll()");

I find it confusing that you use both \n and WriteLine() for newlines. I would prefer to write it like this:

Console.WriteLine();
Console.WriteLine("The following exceptions have been thrown by WaitAll()");

An alternative if you want to be concise is:

Console.Write("\nThe following exceptions have been thrown by WaitAll()\n");

Answer 2

You're not always using var in what I think of as a good way. We should only use the keyword when it's obvious what type the variable is. Consider this line.

Task<bool>[] tasks = new Task<bool>[sites.Count()];

It's completely obvious what tasks is because you're newing it up right there on that line. There's no reason not to simplify by using var.

But here, you have the opposite problem.

var response = client.Execute(request); 

It is not obvious what the return type is, so you shouldn't be using var.

Answer 3

• I don't see a reason, why you prefer the extension method Count() over the Length property of the string[] sites array.

• Comments should describe why something is done and they should be valid / true

  // Construct started tasks
  Task<bool>[] tasks = new Task<bool>[sites.Count()]; 
  

  this comment is

  1. boilerplate code
  2. misleading because neither does the next line construct any task nor started tasks.

• If this should be used by others also, you need to validate the input parameters. In the current state, if e.g sites == null you will get an ArgumentNullException.

• the optional parameter description can be removed, because it isn't used at all

• RubberDuck: Hmm... I can think of a good reason to use the extension instead of the Length property. If the implementation later changes to use a different IEnumerable, you can still call Count. You'd only have to change the code in one place and it'd still work. The other advice is spot on (as always) though.