5

In my controller I need to call a method BlockingHttpRequest() which makes an http request. It is relatively slow running, and blocks the thread.
I am not in a position to refactor that method to make it async.

Is it better to wrap this method in a Task.Run to at least free up a UI/controller thread?
I'm not sure if this really improves anything, or just makes more work.

public class MyController : Controller
{
    public async Task<PartialViewResult> Sync()
    {
        BlockingHttpRequest();
        return PartialView();
    }

    public async Task<PartialViewResult> Async()
    {
        await Task.Run(() => BlockingHttpRequest());
        return PartialView();
    }
}

In practice I have a couple of situations like this, where BlockingHttpRequest() takes 500ms and 5000ms respectively.

I understand that Task.Run() will not make my function return sooner.

I thought that there might be some benefit of increased throughput. By freeing up the controller's thread, does that make it available to other users? This would imply that in MVC there is a difference between controller threads and background worker threads.

Buh Buh
  • 7,443
  • 1
  • 34
  • 61
  • in my experience wrapping something like this in a task boosts the userexperience, because of a responding UI. – Sebastian L Nov 22 '16 at 09:56
  • It doesn't yield until the async has completed since he awaits it @SebastianL – Patrick Hofman Nov 22 '16 at 09:56
  • There is no UI thread. – Patrick Hofman Nov 22 '16 at 09:57
  • Possible duplicate of [When should I use Async Controllers in ASP.NET MVC?](http://stackoverflow.com/q/30566848/11683) – GSerg Nov 22 '16 at 09:59
  • It will indeed improve nothing in this case. – Evk Nov 22 '16 at 10:03
  • Yes, when I said "UI thread" I meant the controllers thread, not the browsers thread. – Buh Buh Nov 22 '16 at 10:06
  • @PatrickHofman I have edited my question to explain. Please would you remove the marked-as-duplicate. – Buh Buh Nov 22 '16 at 10:09
  • You are still awaiting the task, so in the end it doesn't matter. Actually your question is: *Does it matter to use `Task.Run` on synchronous code?* – Patrick Hofman Nov 22 '16 at 10:11
  • Also define 'blocking', because if it blocks the thread, starting a task isn't going to help. – Patrick Hofman Nov 22 '16 at 10:12
  • So you are running SlowAndBlocking on another thread (via Task.Run). Request thread is now free, but what has changed? One additional thread is busy, another is free - nothing has changed. And user still waits for request to complete exactly the same amount of time. – Evk Nov 22 '16 at 10:41
  • @PatrickHofman Not sure how to define blocking. Yes, it blocks the thread. It makes an api call and blocks until it returns. – Buh Buh Nov 22 '16 at 10:42
  • @Evk, yes but are all threads of equal worth? It there a benefit to freeing up the controllers thread so that another user's request can be accepted? Is there increased throughput? – Buh Buh Nov 22 '16 at 10:43
  • 1
    As far as I know, ASP.NET does not have separate thread pool and uses regular thread pool (so, the same pool on which your Task.Run will be scheduled). – Evk Nov 22 '16 at 10:44
  • @Evk is right. Just one thread pool. Using tasks allows scalability since you free up threads, but nothing else. (as said in the duplicate). The user will not notice anything. – Patrick Hofman Nov 22 '16 at 10:46
  • This is the umpteenth version of _"How to use async/await and tasks **for this particular task**"_ this week. How hard can it be for someone to post a low-level Q&A or blog post with understandable language, for which you don't need to read their previous twenty blog posts or Q&As? I cannot find a proper duplicate that explains this specific scenario. – CodeCaster Nov 22 '16 at 10:49
  • In general, almost in all situations, async\await makes sense only if you are doing asynchronous IO (reading files, making database queries, http requests and such things). – Evk Nov 22 '16 at 10:50
  • @PatrickHofman I understand an individual user won't notice the difference; that's fine. You said "allows scalability"; are you implying that there would be improved throughput? Are you implying it is better code? – Buh Buh Nov 22 '16 at 10:52
  • @Evk Yes I am indeed doing those things, but the API I have been provided with is Sync NOT Async. This is that part of the question I would like you to focus on. – Buh Buh Nov 22 '16 at 10:53
  • The duplicate question explains it all IMHO. By freeing the thread you allow more concurrency (since the thread can handle other stuff). But since tasks only really benefit when they are awaiting an external resource (I/O), it doesn't help if the task runs all the time. – Patrick Hofman Nov 22 '16 at 10:54
  • 1
    Yes, I stated that it makes sense when you are doing _asynchronous_ IO. You are not doing any, so no reason to use async\await, especially combined with Task.Run. As you already understand I think, the answer to your question is "no". There is no reason to await Task.Run your slow method, at all. On the other hand, if you have multiple such slow methods, you can run them in parallel via Task.Run - in that case it will make sense (user will receive response faster). – Evk Nov 22 '16 at 10:58
  • You are not moving it to another thread. You run the task on the same thread. – Patrick Hofman Nov 22 '16 at 11:07
  • @PatrickHofman So `Task.Run` does not create a new thread? It just creates the Task state machine on the same thread. That doesn't sound very useful. – Buh Buh Nov 22 '16 at 11:55
  • You can't know if it creates a task or not. That is up to the CLR. – Patrick Hofman Nov 22 '16 at 11:58
  • @DavidOsborne The operation in question is a blocking http request; if that matters. – Buh Buh Nov 22 '16 at 14:05
  • @PatrickHofman `Task.Run` schedules the operation to be run by the thread pool (unless it's marked as long running). It is then up to the thread pool as to whether the operation is run by some existing thread pool thread, or if a new worker thread will be created to handle it. In an ASP application the thread pool is quite likely to already be saturated, so an existing thread is likely to be used. Since we're also freeing another thread pool thread at the same time we're scheduling a new one, there is a *slight* chance that the same thread handles the response, but it's not super likely. – Servy Nov 22 '16 at 14:25
  • @Servy Okay, thanks for that. – Patrick Hofman Nov 22 '16 at 14:26

2 Answers2

3

By freeing up the controller's thread, does that make it available to other users?

Yes, it does.

This would imply that in MVC there is a difference between controller threads and background worker threads.

No, there is not. Because just as you're freeing up a thread pool thread that can now go service other requests, you're also consuming a new thread pool thread that now cannot go service other requests. So the total number of threads able to go work on other requests is the same.

I thought that there might be some benefit of increased throughput.

There is not. You're wasting some time context switching between threads, waiting for the new worker to be scheduled, and other overhead related to the work that you're doing, and you're getting nothing in return.

Servy
  • 202,030
  • 26
  • 332
  • 449
0

Because the method is blocking, moving it onto a different thread achieves nothing. Just wastes one thread instead of another, both of which were created equal.

Furthermore, because a thread is still wasted either way there is no increased scalability or throughput.

Buh Buh
  • 7,443
  • 1
  • 34
  • 61