How can I better my URL checker program?

Question

I made a LinkChecker program which checks URLs from a text file and outputs status into another file like this:

http://valid-url.com;OK
http://invalid-url.com;The remote server returned an error: (404) Not Found.

I have about 1400 URLs in the input file and it takes about 12 minutes to run.

I made it work. In what ways can I achieve rightness and fastness?

public class LinkChecker
{
    static void Main()
    {
        string line = string.Empty;
        string inputFile = @"D:\tmp\links.txt";
        StreamReader file = new StreamReader(inputFile);
        string outputFile = @"D:\tmp\out.txt";

        ClearOutputFile(outputFile);

        while(!string.IsNullOrEmpty(line = file.ReadLine()))
        {
            string currentUrlAndStatus = CheckURL(line);
            if(!string.IsNullOrEmpty(currentUrlAndStatus))
            {
                WriteToFile(currentUrlStatus, outputFile);
            }
        }
    }

    private static string CheckURL(string url)
    {
        string status = string.Empty;

        HttpWebRequest request = (HttpWebRequest)WebRequest.Create(url);
        request.Method = "HEAD";
        request.Proxy = null;

        try
        {
            using(HttpWebResponse response = (HttpWebResponse)request.GetResponse())
            {
                status = response.StatusCode.ToString();
            }
        }
        catch (WebException ex)
        {
            status = ex.Status;
        }

        return url + ";" + status;
    }

    private static void ClearOutputFile(string file)
    {
        using (Filestream fileStream = File.Open(file, FileMode.Open)
        {
            fileStream.SetLength(0);
        }
    }

    private static void WriteToFile(string message, string filename)
    {
        using(var writer = new StreamWriter(filename, true))
        {
            writer.WriteLine(message);
        }
    }
}

Answer 1

You forgot using using in your StreamReader

    using(StreamReader file = new StreamReader(inputFile)){
    string outputFile = @"D:\tmp\out.txt";

    ClearOutputFile(outputFile);

    while(!string.IsNullOrEmpty(line = file.ReadLine()))
    {
        string currentUrlAndStatus = CheckURL(line);
        if(!string.IsNullOrEmpty(currentUrlAndStatus))
        {
            WriteToFile(currentUrlStatus, outputFile);
        }
    }
  }

Use the var keyword for local variables, let the compiler figure out the type for you. Type inference helps in refactoring.

 var line = string.Empty;
 var inputFile = @"D:\tmp\links.txt";
 using(var file = new StreamReader(inputFile)){
 //...
 }
 var outputFile = @"D:\tmp\out.txt";
 // etc..

Answer 2

It takes 12 minutes because it has to wait for a request response on every URL, one by one. If you want it to go faster, it's better to load all URLs from your file into an array of strings first, then run a number of threads so you can check multiple URLs simultaneously. Wait for all threads to finish before writing it back to a file.

More on threading in C# here: http://msdn.microsoft.com/en-us/library/aa645740%28v=vs.71%29.aspx

Answer 3

Read entire file at first then start async web requests, it shouldn't take more than a minute or two. Time depends on file size, your and server's bandwidth etc.

Why Async? There are servers slower/faster than others, or even some servers may return a timeout which will take "much time", to avoid them you should start web requests async. and handle their completed event to write results to file. IMFO writing line by line or first in-memory and flush will effect performance

Answer 4

Generally IO is the slowest bottlenecks in programming, reading the file from the harddisk and making the web requests.

For the file itself, I would recommend reading the entire file as one operation, then splitting it into an array of lines that you can then check each.

For the web-requests, I would do what the others have suggested, make threads/tasks so you can call several async requests at the same time. I feel this will have the biggest impact on speed of your program. Unfortunately it is not as simple as creating as many tasks as you can and let them all run at once. Each core in your cpu can handle a single task, and it will cycle through them based on their priority and other tasks waiting to be worked on, giving each a bit of cpu cycles before moving onto the next. I would suggest starting with 2-4 threads/tasks and see if increasing or decreasing them helps performance. Using the Task Parallel Library (suggested by Snowbody) can be a useful way of automating this.

As ANeves points out, Web requests will likely be your main culprit for speed issues since they can take seconds to come back with an answer (or 30+ seconds if waiting for them to timeout).

A good way to test this is to use the StopWatch class and make a test project that calls your class numerous times and logs the average results.

Write the sum of all the results to the file at the same time when completed.

Those will likely have the biggest impact (I would focus on the web requests first, then file IO operations next). Other optimizations may help as well, but likely very small impacts or none at all, such as using StringBuilder vs string for the string concatenations. Also other improvements depend on the data. If likely to have duplicate URL's in the data, it might be worth checking against a list of already processed ones to save you time. Another possibility is if they could have badly formed url's, IO will still likely be slower than checking against an in-memory list. Hope it helps!

Answer 5

With the parallelization I suggest below I expect the process to get fast enough that async does not matter much.
But @IlmariKaronen's suggestion is very good too:

Instead of threads, it might be better to use async web requests, as described here and here.

---

My suggestions follow.

Parallelization

You can do some parallelization with Parallel.ForEach. As simple as it gets.

string[] urls = File.ReadAllLines(inputFile);
Parallel.ForEach(urls, url =>
{
    // Probe.
});

Output

Although IO is usually the heaviest, writing 1400 lines to file is not what is taking 12 minutes. Still, you don't need to keep the file open and write as you go.

Instead, you can write to a StringBuilder sb, and in the end simply use File.WriteAllText(outputFile, sb.ToString());.

This method creates or overwrites the file, so it will have the additional improvement that you can remove the method ClearOutputFile.

Misc

• This if is never false because your method always returns something (at least :<status>), so you can remove it: if(!string.IsNullOrEmpty(currentUrlAndStatus))
• You can make the variable status a HttpStatusCode instead.
• When you use streams you should dispose them... using using is the correct and idiomatic way to do it.