11

Inside a c# project I'm making some calls to a web api, the thing is that I'm doing them within a loop in a method. Usually there are not so many but even though I was thinking of taking advantage of parallelism.

What I am trying so far is

public void DeployView(int itemId, string itemCode, int environmentTypeId)
{
    using (var client = new HttpClient())
    {
        client.BaseAddress = new Uri(ConfigurationManager.AppSettings["ApiUrl"]);
        client.DefaultRequestHeaders.Accept.Clear();
        client.DefaultRequestHeaders.Accept.Add(new MediaTypeWithQualityHeaderValue("application/json"));

        var agents = _agentRepository.GetAgentsByitemId(itemId);

        var tasks = agents.Select(async a =>
            {
                var viewPostRequest = new
                    {
                        AgentId = a.AgentId,
                        itemCode = itemCode,
                        EnvironmentId = environmentTypeId
                    };

                var response = await client.PostAsJsonAsync("api/postView", viewPostRequest);
            });

        Task.WhenAll(tasks);
    }
}

But wonder if that's the correct path, or should I try to parallel the whole DeployView (i.e. even before using the HttpClient)

Now that I see it posted, I reckon I can't just remove the variable response as well, just do the await without setting it to any variable

Thanks

Jasper
  • 11,590
  • 6
  • 38
  • 55
mitomed
  • 2,006
  • 2
  • 29
  • 58
  • 2
    Well actually that's a good direction you are going. But you forgot the most important part. You need to *await* the results e.g. *await Task.WhenAll* but then you need to add the 'async' keyword to your DeployView function. You better take a deep tour of the [async/await](https://msdn.microsoft.com/en-us/library/hh191443.aspx) paradigm. – ckruczek Jul 13 '15 at 08:05
  • 1
    so whats the problem/exception you are getting facing? I agree with ckruczek as well, nothing wrong with the direction you are taking..also, do you want to get the responses at all? –  Jul 13 '15 at 08:06
  • I would like to get the responses, yes. But not sure how to use them if they are all Ok – mitomed Jul 13 '15 at 08:13
  • "try to parallel the whole DeployItem" What do you mean by that? – usr Jul 13 '15 at 08:20
  • @usr, I was wondering if the point would be to create the htppclient within the tasks themselve. Sorry, I put DeployItem and it was supposed to be DeployView – mitomed Jul 13 '15 at 08:21

3 Answers3

6

Usually there is no need to parallelize the requests - one thread making async requests should be enough (even if you have hundreds of requests). Consider this code:

var tasks = agents.Select(a =>
        {
            var viewPostRequest = new
                {
                    AgentId = a.AgentId,
                    itemCode = itemCode,
                    EnvironmentId = environmentTypeId
                };

            return client.PostAsJsonAsync("api/postView", viewPostRequest);
        });
    //now tasks is IEnumerable<Task<WebResponse>>
    await Task.WhenAll(tasks);
    //now all the responses are available
    foreach(WebResponse response in tasks.Select(p=> p.Result))
    {
        //do something with the response
    }

However, you can utilize parallelism when processing the responses. Instead of the above 'foreach' loop you may use:

Parallel.Foreach(tasks.Select(p=> p.Result), response => ProcessResponse(response));

But TMO, this is the best utilization of asynchronous and parallelism:

var tasks = agents.Select(async a =>
        {
            var viewPostRequest = new
                {
                    AgentId = a.AgentId,
                    itemCode = itemCode,
                    EnvironmentId = environmentTypeId
                };

            var response = await client.PostAsJsonAsync("api/postView", viewPostRequest);
            ProcessResponse(response);
        });
await Task.WhenAll(tasks);   

There is a major difference between the first and last examples: In the first one, you have one thread launching async requests, waits (non blocking) for all of them to return, and only then processing them. In the second example, you attach a continuation to each Task. That way, every response gets processed as soon as it arrives. Assuming the current TaskScheduler allows parallel (multithreaded) execution of Tasks, no response remains idle as in the first example.

*Edit - if you do decide to do it parallel, you can use just one instance of HttpClient - it's thread safe.

shay__
  • 3,815
  • 17
  • 34
  • Async IO does not make the IO faster. It's unrelated to speed of that single IO. He can *only* go faster by parallelizing. – usr Jul 13 '15 at 08:29
  • I never said async makes the IO faster. Posting a single async request is relatively fast (no need to wait for a response), fast enough for one thread to make thousands of these in no time. That's why I emphasized 'Usually'. – shay__ Jul 13 '15 at 08:31
5

What you're introducing is concurrency, not parallelism. More on that here.

Your direction is good, though a few minor changes that I would make:

First, you should mark your method as async Task as you're using Task.WhenAll, which returns an awaitable, which you will need to asynchronously wait on. Next, You can simply return the operation from PostAsJsonAsync, instead of awaiting each call inside your Select. This will save a little bit of overhead as it won't generate the state-machine for the async call:

public async Task DeployViewAsync(int itemId, string itemCode, int environmentTypeId)
{
    using (var client = new HttpClient())
    {
        client.BaseAddress = new Uri(ConfigurationManager.AppSettings["ApiUrl"]);
        client.DefaultRequestHeaders.Accept.Clear();
        client.DefaultRequestHeaders.Accept.Add(
                   new MediaTypeWithQualityHeaderValue("application/json"));

        var agents = _agentRepository.GetAgentsByitemId(itemId);
        var agentTasks = agents.Select(a =>
        {
            var viewPostRequest = new
            {
                AgentId = a.AgentId,
                itemCode = itemCode,
                EnvironmentId = environmentTypeId
            };

            return client.PostAsJsonAsync("api/postView", viewPostRequest);
        });

        await Task.WhenAll(agentTasks);
    }
}

HttpClient is able to make concurrent requests (see @usr link for more), thus I don't see a reason to create a new instance each time inside your lambda. Note that if you consume DeployViewAsync multiple times, perhaps you'll want to keep your HttpClient around instead of allocating one each time, and dispose it once you no longer need its services.

Community
  • 1
  • 1
Yuval Itzchakov
  • 146,575
  • 32
  • 257
  • 321
4

HttpClient appears to be usable for concurrent requests. I have not verified this myself, this is just what I gather from searching. Therefore, you don't have to create a new client for each task that you are starting. You can do what is most convenient to you.

In general I strive to share as little (mutable) state as possible. Resource acquisitions should generally be pushed inwards towards their usage. I think it's better style to create a helper CreateHttpClient and create a new client for each request here. Consider making the Select body a new async method. Then, the HttpClient usage is completely hidden from DeployView.

Don't forget to await the WhenAll task and make the method async Task. (If you do not understand why that is necessary you've got some research about await to do.)

Community
  • 1
  • 1
usr
  • 168,620
  • 35
  • 240
  • 369
  • Thanks for your insightful answer, as @YuvalItzchakov metnioned I think it makes more sense for me in this case keeping the client, given it is useable for concurrent requests, as this method will be called multiple times – mitomed Jul 13 '15 at 08:41