Question

I have created the following simple HttpListener to serve multiple requests at the same time (on .NET 4.5):

class Program {

    static void Main(string[] args) {

        HttpListener listener = new HttpListener();
        listener.Prefixes.Add("http://+:8088/");
        listener.Start();
        ProcessAsync(listener).ContinueWith(task => { });
        Console.ReadLine();
    }

    static async Task ProcessAsync(HttpListener listener) {

        HttpListenerContext ctx = await listener.GetContextAsync();

        // spin up another listener
        Task.Factory.StartNew(() => ProcessAsync(listener));

        // Simulate long running operation
        Thread.Sleep(1000);

        // Perform
        Perform(ctx);

        await ProcessAsync(listener);
    }

    static void Perform(HttpListenerContext ctx) {

        HttpListenerResponse response = ctx.Response;
        string responseString = "<HTML><BODY> Hello world!</BODY></HTML>";
        byte[] buffer = Encoding.UTF8.GetBytes(responseString);

        // Get a response stream and write the response to it.
        response.ContentLength64 = buffer.Length;
        Stream output = response.OutputStream;
        output.Write(buffer, 0, buffer.Length);

        // You must close the output stream.
        output.Close();
    }
}

I use Apache Benchmark Tool to load test this. When I make a 1 request, I get the max wait time for a request as 1 second. If I make 10 requests, for example, max wait time for a response goes up to 2 seconds.

How would you change my above code to make it as efficient as it can be?

Edit

After @JonSkeet's answer, I changed the code as below. Initially, I tried to simulate a blocking call but I guess it was the core problem. So,I took @JonSkeet's suggestion and change that to Task.Delay(1000). Now, the below code gives max. wait time as approx. 1 sec for 10 concurrent requests:

class Program {

    static bool KeepGoing = true;
    static List<Task> OngoingTasks = new List<Task>();

    static void Main(string[] args) {

        HttpListener listener = new HttpListener();
        listener.Prefixes.Add("http://+:8088/");
        listener.Start();
        ProcessAsync(listener).ContinueWith(async task => {

            await Task.WhenAll(OngoingTasks.ToArray());
        });

        var cmd = Console.ReadLine();

        if (cmd.Equals("q", StringComparison.OrdinalIgnoreCase)) {
            KeepGoing = false;
        }

        Console.ReadLine();
    }

    static async Task ProcessAsync(HttpListener listener) {

        while (KeepGoing) {
            HttpListenerContext context = await listener.GetContextAsync();
            HandleRequestAsync(context);

            // TODO: figure out the best way add ongoing tasks to OngoingTasks.
        }
    }

    static async Task HandleRequestAsync(HttpListenerContext context) {

        // Do processing here, possibly affecting KeepGoing to make the 
        // server shut down.

        await Task.Delay(1000);
        Perform(context);
    }

    static void Perform(HttpListenerContext ctx) {

        HttpListenerResponse response = ctx.Response;
        string responseString = "<HTML><BODY> Hello world!</BODY></HTML>";
        byte[] buffer = Encoding.UTF8.GetBytes(responseString);

        // Get a response stream and write the response to it.
        response.ContentLength64 = buffer.Length;
        Stream output = response.OutputStream;
        output.Write(buffer, 0, buffer.Length);

        // You must close the output stream.
        output.Close();
    }
}
Was it helpful?

Solution

It looks to me like you'll end up with a bifurcation of listeners. Within ProcessAsync, you start a new task to listen (via Task.Factory.StartNew), and then you call ProcessAsync again at the end of the method. How can that ever finish? It's not clear whether that's the cause of your performance problems, but it definitely looks like an issue in general.

I'd suggest changing your code to be just a simple loop:

static async Task ProcessAsync(HttpListener listener) {
    while (KeepGoing) {
        var context = await listener.GetContextAsync();
        HandleRequestAsync(context);         
    }
}

static async Task HandleRequestAsync(HttpListenerContext context) {
    // Do processing here, possibly affecting KeepGoing to make the 
    // server shut down.
}

Now currently the above code ignores the return value of HandleRequestAsync. You may want to keep a list of the "currently in flight" tasks, and when you've been asked to shut down, use await Task.WhenAll(inFlightTasks) to avoid bringing the server down too quickly.

Also note that Thread.Sleep is a blocking delay. An asynchronous delay would be await Task.Delay(1000).

Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top