CODEWITHSUNDEEP
c#.net-coreasync-awaittask
Quade
Quade

Reputation: 165

Is it bad practice to have async call within Task.Run?

I have a C# console app processing around 100,000 JSON messages from RabbitMQ every 1 min

After getting each/a bunch of messages from RabbitMQ I then call

 await Task.Run(async () =>
 {
    //do lots of CPU stuff here, including 2 external API calls using await async call
 }

Everything I've read says use await Task.Run for CPU bound operations. And use await async for the HTTP external calls.

If I change it to:

await Task.Run(() =>

Then it complains as I have an async API call in the lines below, so it needs the async keyword in the Task.Run statement.

There are about 2000+ (complex if then business rules) lines of code in this section, and the sometimes the API call is not needed.

So I'm faced with either a massive restructure of the application, with lots of testing needed, or if its ok to do API calls alongside the CPU bound operations then I'll leave it as is.

To summarise, is this bad practice, or is it ok to have CPU bound work and API calls inside the same task? The task is processing one JSON message.

Upvotes: 1

Views: 1653

Answers (2)

tmaj
tmaj

Reputation: 34967

General

(...) use await Task.Run for CPU bound operations. And use await async for the HTTP external calls.

This advice comes from the fact that if you run code that doesn't 'let go' enough then you may not get a lot of the benefit Tasks give because the current thread / thread pool just cannot handle the work. In the extreme case when you run 100% synchronous code you won't get any parallelism because the current thread cannot let go and cannot do any other work - your tasks would be executed sequentially. It's important to remember that this is not a style issue; running busy synchronous code with Tasks just work well in some scenarios. In this sense the problem polices itself: if you structure the solution incorrectly it doesn't do what you need.

If you run a mixture of busy and waiting then Task.Run may or may not be great and it will depend on the specific workload. If it works for you, it's fine - you're not doing anything incorrect.

Generally the picture is nuanced and tasks can be and are used to do all kinds of jobs. In certain circumstances the situation is clear cut - e.g. if you run long running work in the UI thread you will lock the UI which is bad or if you have (long-running) busy synchronous code. It's worth keeping in mind that this has been a problem before C# had Tasks.

BTW. If you look at the reference documentation for Task.WhenAll Method it contains examples with both I/O (ping) and CPU (dummy for loop) style work. Yes, these are toy examples but it shows it isn't incorrect to run both types of work with tasks.

Parallel.ForEachAsync?

If you can use .NET 6, Parallel.ForEachAsync could improve performance of your solution and/or make the code look cleaner. Example on Twitter (as picture!).

Upvotes: 0

Stephen Cleary
Stephen Cleary

Reputation: 456377

Everything I've read says use await task.run for cpu bound operations . And use await async for the http external calls

The general guideline is to use async/await for I/O. Task.Run is useful for CPU-bound operations if you need to offload them from a UI thread. For example, in server scenarios such as ASP.NET, you wouldn't want to use Task.Run for CPU-bound code. This is because ASP.NET already schedules your code on a separate thread pool thread.

In your case, you have a Console application, which doesn't have a UI thread. But it also doesn't have that automatic scheduling onto a thread pool thread that ASP.NET gives you.

if its ok to do api calls alongside the cpu bound operations then i'll leave it as is.

This is fine either way. Since the code is awaiting the Task.Run, it won't continue (presumably processing the next message) until the operation completes on another thread pool thread. So the Task.Run isn't helping much, but it isn't hurting much, either.

If you need more performance - specifically, processing messages concurrently - then you should look into something like TPL Dataflow or System.Threading.Channels that would allow you to replace the Task.Run with a queue of work that can run in parallel. That would give you something more like what ASP.NET provides out of the box.

Upvotes: 5

Related Questions