9
votes

I've been trying to learn C#'s async with HttpClient and have come across an issue where I can't figure out what's happening.

What I want to do:

  • Send multiple HttpClient requests in one go and have the responses processed individually as soon as they return. This process is repeated in a loop forever (i.e. a new batch of requests sent off every few seconds). The processing done on the results are not CPU intensive and take a few milliseconds at most per response.
  • Quantity is 5-50 posts in any one instance
  • Don't care about the order they come back but each request/response must process as fast as possible without blocking.

The problem:

  • Whenever I send a list of requests together, the responses seem to take ever so slightly longer, but they should each be approximately the same. e.g.:

    Time taken: 67.9609 milliseconds
    Time taken: 89.2413 milliseconds
    Time taken: 100.2345 milliseconds
    Time taken: 117.7308 milliseconds
    Time taken: 127.6843 milliseconds
    Time taken: 132.3066 milliseconds
    Time taken: 157.3057 milliseconds

When they should all be just around 70ms... This repeats for every new batch; the first one is quick, then each consecutive one slows down.

My Code
There are 3 main parts. I've simplified it as best I can to focus on where the problem is occuring (Downloader.cs is what I timed)

  • Downloader.cs This contains the method that does the actual http query:

    public async Task<string> DownloadData(string requestString)
    {
        HttpResponseMessage response;
        response = await this.httpClient.PostAsync( url, new StringContent( requestString ));
        string returnString = await response.Content.ReadAsStringAsync();
        return returnString;
    }
    
  • QueryAgent.cs This initialises a new Downloader and contains the main async looping caller function and the async method it calls in a loop.
    I need to send of new requests as soon as possible depending on the result of the last set of responses (basically, the rate at which requests go out can change so I can't do a Task.delay() at the end of the while loop)

    public async Task DownloadLoopAsync() 
    {
        while ( this.isLooping )
        {
            // I put this here to stop it using lots of cpu. 
            // Not sure if it is best practice.
            // advice on how to do this kind of loop would be appreciated.
            await Task.Delay( 5 );  
    
            foreach ( string identifier in this.Identifiers )
            {
                string newRequestString = this.generateRequestString(identifier );
                Task newRequest = RequestNewDataAsync(newRequestString);
            }
        }
    }     
    
    public async Task RequestNewDataAsync(string requestString)
    {
        string responseString = await this.downloader.DownloadData(requestString);
        this.ProcessedData = this.ProcessData(responseString);
    }
    
  • Program.cs This is just a console program that calls a Task once on the async function in QueryAgent and then a Console.Readline() at the end to stop the program exiting. I initialize the HttpClient in here too. I call DownloadLoop() once, as such:

    QueryAgent myAgent = new QueryAgent(httpClient);
    Task dataLoop = myAgent.DownloadLoopAsync();   
    Console.Readline();
    

I have a suspicion it has something to do with the way that I'm calling a new Task in the for loop in DownloadLoopAsync(), however this doesn't make sense because I assumed that each new Task would be processed by itself.

Any advice is greatly appreciated as I have been hitting brick wall after brick wall trying to get this working as intended.

thank you.

EDIT 1
Just had a mess around in the calling Task loop. I changed:

foreach(string identifier in this.Identifiers)
{
    string newRequestString = this.generateRequestString(identifier );
    Task newRequest = RequestNewDataAsync(newRequestString);
}

to

foreach(string identifier in this.Identifiers)
{
    string newRequestString = this.generateRequestString(identifier );
    Task newRequest = RequestNewDataAsync(newRequestString);
    await Task.Delay(1);
}

which now makes it work as expected:

Time taken: 71.0088 milliseconds
Time taken: 65.0499 milliseconds
Time taken: 64.0955 milliseconds
Time taken: 64.4291 milliseconds
Time taken: 63.0841 milliseconds
Time taken: 64.9841 milliseconds
Time taken: 63.7261 milliseconds
Time taken: 66.451 milliseconds
Time taken: 62.4589 milliseconds
Time taken: 65.331 milliseconds
Time taken: 63.2761 milliseconds
Time taken: 69.7145 milliseconds
Time taken: 63.1238 milliseconds

Now I'm even more confused why this appears to work!

EDIT 2
The timing code exists as follows inside Downloader.cs:

public async Task<string> DownloadData(string requestString)
{
    HttpResponseMessage response;
    Stopwatch s = Stopwatch.StartNew();
    response = await this.httpClient.PostAsync( url, new StringContent( requestString ));
    double timeTaken = s.Elapsed.TotalMilliseconds;
    Console.WriteLine( "Time taken: {0} milliseconds", timeTaken );
    string returnString = await response.Content.ReadAsStringAsync();
    return returnString;
}

Although the await Task.Delay( 1 ); seems to "do the trick", it is not an ideal solution nor do I understand why it's working - perhaps somehow it gives the CPU time to initialize the RequestNewDataAsync Task and prevents a lockup of some sort concurring? I can only guess... It also means that it limits the loop to 1000Hz, which is fast, but also a limit since Task.Delay() only accepts integer values, of which 1 is the minimum.

EDIT 3
Doing a bit more reading (I am still relatively new to all this!), maybe this is something best left to using the Threadpool (10 to 100s of short lived tasks)? However would this would create excessive (1MB per new task?) overhead; or is that something different again.

EDIT 4
Did a little bit more experimenting putting the Task.Delay(1) in different places.

  • [slowdown occurs] Placing before or after httpClient.PostAsync() in Downloader.cs
  • [slowdown occurs] Placing before or after the await call in RequestNewDataAsync()
  • [no slowdown / expected performance] Placing before or after the Task call in DownloadLoopAsync()
2
where are you sending the requests to? is it possible you are just putting load of the server, hence the slower response? - Ewan
HttpClient cannot be used concurrently, it is not thread-safe. You also never await the resulting task from RequestNewDataAsync so you are sending more requests before the earlier ones have completed, putting more and more load on the server. - Lukazoid
your approach seems slightly non-standard. I would suggest you have a list of tasks which you add to and then await.all rather than the loop - Ewan
HttpClient should be ok as long as you make a new one each time surely - Ewan
@Anthony: Have you set ServicePointManager.DefaultConnectionLimit? Also, please post a complete minimal repro. - Stephen Cleary

2 Answers

6
votes

When you do an await, the subsequent code gets executed only after what you 'awaited' gets completed.

To send multiple queries concurrently, you have to create new Task for each query and then await all of them together.

var tasks = new List<Task>();

foreach (string identifier in Identifiers)
{
    var newRequestString = generateRequestString(identifier);
    var newRequest = RequestNewDataAsync(newRequestString);
    tasks.Add(newRequest);
}

Task.WhenAll(tasks); // synchronous wait for all tasks to complete
1
votes

Most likely you are just saturating your network or the remote server with requests. Your infinite loop is not waiting for the previous batch to complete before starting the next batch, so over time you are issuing batches faster than they can complete, causing a backlog.

Here's a modified loop that waits for a batch to complete before starting the next batch. Notice it no longer awaits a Task.Delay and instead awaits the collection of download tasks.

public async Task DownloadLoopAsync() 
{
    while ( this.isLooping )
    {
      var tasks = this
        .Identifiers
        .Select(id => RequestNewDataAsync(this.generateRequestString(id));

      await Task.WhenAll(tasks);
    }
}

Note you have a race condition in how you store the results of the processed data. This may or may not be an issue for you:

public async Task RequestNewDataAsync(string requestString)
{
    string responseString = await this.downloader.DownloadData(requestString);

    // Note: you have a race condition here.
    // multiple downloads could complete simultaneously and try to concurrently
    // assign to ProcessedData.  This may or may not be an issue for you.
    this.ProcessedData = this.ProcessData(responseString);
}

Also I modified your main program to await the final batch to complete after the user presses a key:

QueryAgent myAgent = new QueryAgent(httpClient);
Task dataLoop = myAgent.DownloadLoopAsync();   
Console.Readline();
myAgent.isLooping = false;
dataLoop.Wait(); // let the last set of downloads complete