How to optimize multiple web request in C#?

Question

I have to check around 10 url (sometimes more) in a WPF app, and display the result (just "is connected", or "can't connect".

I've done something that works, with the code below :

foreach (string url in urlList)
{
    CheckConnection(url); 
}

static async void CheckConnection(string url)
{
    using (WebClient client = new WebClient())
    {
        bool succeed = true;
        try
        {
            await Task.Run(() => client.DownloadData(url));
        }
        catch
        {
            succeed = false;
        }

        if (succeed){ 
            //display "is connected"
        }
        else{
            //display "can't connect"
        }
    }
}

The problem is that it takes too long to check everything, i dont know why, it seems that it takes way more time than it should. How can I optimize this? Is what I do the best way to do this?

As opposed to downloading all the data can you just post a HEAD to request to it and see if you get a valid response. https://stackoverflow.com/questions/3268926/head-with-webclient — Ryan Thomas, Jan 20 '20 at 15:07
Avoid using `async void` methods. Also, if `url` doesn't exist (404 response), the delay and wait can be significant. You can also try to cancel an operation by timeout. Another option is to use [`DownloadDataTaskAsync`](https://learn.microsoft.com/en-us/dotnet/api/system.net.webclient.downloaddatataskasync?view=netframework-4.8#System_Net_WebClient_DownloadDataTaskAsync_System_String_) method — Pavel Anikhouski, Jan 20 '20 at 15:07
You should check out events. Any time this event is fired off, you could fire off "connected". https://learn.microsoft.com/en-us/dotnet/standard/events/#events — LinkedListT, Jan 20 '20 at 15:09
@PavelAnikhouski What should i use instead of async void methods? If i dont use it, i can't use my app untill every requests are checked. — Axel, Jan 20 '20 at 15:10
You should use `async Task` return type instead of `async void`, have a look [here](https://stackoverflow.com/questions/45447955/why-exactly-is-void-async-bad) — Pavel Anikhouski, Jan 20 '20 at 15:11
Please use HttpClient in any case - as long as you have the ability to do so. — johannespartin, Jan 20 '20 at 15:25

Mark Davies · Accepted Answer · 2020-01-20T15:31:54.930

You can use Task.WhenAll on an array of Tasks: https://learn.microsoft.com/en-us/dotnet/api/system.threading.tasks.task.whenall?view=netframework-4.8

Creates a task that will complete when all of the supplied tasks have completed.

Here's an example of what I mean:


var tasks = new List<Task>();
using (WebClient client = new WebClient())
{
    foreach (string url in urlList)
    {
        tasks.Add(client.DownloadData(url)));
    }

    await Task.WhenAll(tasks);
}

This will essentially start all of the requests then wait for them all to complete before carrying on, means that you're not making one request, waiting, making a second request, waiting, etc....

Just some notes about your code:

You should not return void when doing asyncronous code

Here's some information: https://learn.microsoft.com/en-us/dotnet/csharp/programming-guide/concepts/async/async-return-types#BKMK_VoidReturnType

You use the void return type in asynchronous event handlers, which require a void return type. For methods other than event handlers that don't return a value, you should return a Task instead, because an async method that returns void can't be awaited. Any caller of such a method must be able to continue to completion without waiting for the called async method to finish, and the caller must be independent of any values or exceptions that the async method generates.

HttpClient is the new API

WebClient I think is the old way of doing things? C# moves quickly these days: https://stackoverflow.com/a/23959136/1703915 it has similar performance and you can use it in a very similar way

You should probably check status code rather than catching exceptions

I could write an essay about allowing exceptions to create a control flow, but for now let's keep it simple, http status codes have a way of determining whether they are successful or not - use them.

Hope this helps

I can't edit as the change is too small, `HttpClient is the new API`, small spelling mistake. — ColinM, Jan 20 '20 at 15:22
Edited :D I swear this wireless keyboard I have acts up sometimes — Mark Davies, Jan 20 '20 at 15:32
@Axel: You shouldn't use a background thread to download remote data in .NET in 2020. This is certainly not how to "optimize" your code. — mm8, Jan 20 '20 at 15:55
@mm8 But using a background thread is the only way for me to be able to use my app when data is downloading isnt it? What i'm supposed to do? — Axel, Jan 21 '20 at 09:08
I think what @mm8 is saying is that the `Task.Run` that you have in your code is probably better replaced with `GetByteArrayAsync` (from HttpCient) as his code snippet suggests, `Task.Run` will spin up a new thread where as the async version is.... a little more complicated but probably more efficient because your not spinning up numerous threads. — Mark Davies, Jan 21 '20 at 09:25

score 1 · Answer 2 · answered Jan 20 '20 at 15:45

You should use the HttpClient API and invoke the requests asynchronously:

static async Task<bool> CheckConnection(List<string> urlList)
{
    bool succeed = true;
    using (HttpClient client = new HttpClient())
    {
        var tasks = urlList.Select(x => client.GetByteArrayAsync(x)).ToArray();
        try
        {
            await Task.WhenAll(tasks);
        }
        catch
        {
            succeed = false;
        }
    }
    return succeed;
}

You don't need a thread to perform a web request. It's a waste of resources.

How to optimize multiple web request in C#?

2 Answers2