1

I have a 1 core machine and I would like to improve my code performance using async and await. The code has two major parts. the first an IO operation. (reading from azure service bus queue for multiple sessionId), the second, processing the data - CPU heavy. I have wrapped the DequeueAsync method in an async method which returns a Task:

private async Task<SomeReturnValue> AcceptFromQueueAsync(){
  try{ 
       SomeReturnValue result = await DequeueAsync.configureAwait(false);
       return SomeReturnValue;  
  }
  catch{
   //logging and stuff
  }

The CPU heavy method is sync: DoSyncWork() Since the second part is CPU heavy I don't want to use parallelism (actually can't... ) The CPU part can only start when the IO part finishes. Given the above is the following implementation the way to go with 1 cpu machine?

private void AcceptAndProcessWrapper(){
    //Count is some cosnt defined outside the method
    _acceptTasks = new List<Task<SomeReturnValue>>();
     for (var i = 0; i < Count; i++)
     {
                    _acceptTasks.Add(AcceptFromQueueAsync());
      }
      //The use of list is for convince in processing  
      Task.WaitAll(_acceptTasks.ToArray());
      //The CPU part 
       Task.Run(DoSyncWork).Wait();
 }

I know that I could have not use the Task.Run() for the sync part but I want to allow feature implementation on multiple cores (By starting several Tasks using Task.Run() and holding them in an array) Will the above implementation. (The multiple calls to async method that returns a task) improve the performance? Or should I start a new Task for each async call e.g Task.Run(AcceptFromQueueAsync)?

hquinn
  • 100
  • 1
  • 9

4 Answers4

3

The code that you have will indeed do the asynchronous dequeuing in parallel. It will start all of the asynchronous tasks at once and then wait until they are all done to continue.

Adding an extra call to Task.Run won't help. Starting an asynchronous task in another thread is just adding extra overhead for no added value. Task.Run should be used to run CPU bound work in another thread, not asynchronous work in another thread.

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

Speaking of performance with Task(s) (if you use it in future with DoSyncWork) you should keep also in mind that Task.Run uses ThreadPool by default, but the threads from Thread pool are rather expected to do some fast and small work. If the tasks are really some "heavy" ones, you should consider running them in newly created threads, which might be also forced with the following:

new Task(() => { }, TaskCreationOptions.LongRunning);

EDIT:

After some talk in commeting below, I would want to clear the stuff up a bit more.

Currently, Task.Run(DoSyncWork).Wait(); really does not have any sense (even more, it might take additional time to run the task on another thread (even from ThreadPool ones), but if you want to use several cores in future, you obviously should move that task to another thread to run (and if it's really CPU-heavy, then consider running the task as "LongRunning") (if it's of course needed for your app (not sure what's the type of app you have)). In that case, there is still no need to call .Wait() there instantly.

Agat
  • 4,577
  • 2
  • 34
  • 62
  • The work *isn't* long running. Starting an asynchronous task takes almost no time at all, even if it's a long time before it finishes. There is no need to manually construct a task such as this, nor is there a need for it be "long running". – Servy Nov 12 '13 at 18:21
  • 1
    Well, that's pretty strange that a person with such big rating says such words. ThreadPool (besides of its advantages) has also disadvantages and that's pretty widely known that if ThreadPool runs out of threads and creates new Threads it might even take up to 500ms(!) which is incomparable with new Thread creation time. Please, read also the following article: http://stackoverflow.com/questions/9200573/threadpool-queueuserworkitem-vs-task-factory-startnew. – Agat Nov 12 '13 at 18:31
  • All of that is *irrelevant* to the situation here. **The operation is not long running**. It will take only a handful of microseconds to run, because it is just *starting the task* and **not** waiting for it to complete. Putting that work into a thread pool thread is entirely unnecessary in the first place, and if you choose to do it, for whatever reason, it most certainly isn't going to take long; it's going to be *blazingly fast*. – Servy Nov 12 '13 at 18:33
  • Everything is relevant to development. And if you advice something to people, you should consider that they are going to use it in future, but not only in this very place. I don't see why I can not point the user about performance nuances, which he can use futher. If he interested in the stuff I told, he always can clarify that with comments. If not -- that might be helpful for others. I prefer to provide people a fishing rod instead of just feeding them with fish. – Agat Nov 12 '13 at 18:42
  • This has *nothing to do with the question at all*. It is not answering the question. Answers on SO need to *answer the question they are posted to*. It is not appropriate to post an answer that has random unrelated information, even if that information isn't wrong. Providing random unrelated information isn't helping any readers solve their own problems, so your argument is completely invalid. How are you expecting the reader to use the information in this answer to solve the problem that they have, even indirectly? – Servy Nov 12 '13 at 18:47
  • "Nothing" really do your comments of such type . As for the "how are me expecting": the user asked of performance of his code, he also mentioned that he is expecting to use multiple cores in future and, what's the most important(!), he said that DoSyncWork is heavy(!) CPU method, which is obvious, that it must be called not on Thread Pool if he decides to call it in parallel in future. – Agat Nov 12 '13 at 19:01
  • But the question isn't about calling the synchronous method using `Run`, it's about calling the *asynchronous* method using `Run`. The asynchronous method shouldn't be called using `Run` at all. He specifically said that the synchronous method *can't* be run in parallel; that is not possible, so there will no be an parallelization of it in the future. It will, now and in the future, be run in the currently executing thread, not in a thread pool thread, or another background thread. – Servy Nov 12 '13 at 19:07
  • Well, the question initially is related even to async/await (which is called even before `Task.WaitAll`, which is even called before the "famous" `Run`), so I am not sure why you can mention it in your answer, but another users can't. – Agat Nov 12 '13 at 19:11
  • *"I know that I could have not use the Task.Run() for the sync part but I want to allow feature implementation on multiple cores (By starting several Tasks using Task.Run() and holding them in an array) "* -- isn't you considering that the user wrote that somewhy in here, and that the "somewhy" is directly related to the `Run`? I am not sure that the guy just wanted to boast and share his unique decision to the world, but rather, first, to get advice on his decisions. – Agat Nov 12 '13 at 19:15
  • But instead of helping this one and (possibly) other users on other questions, you put minuses and spend time on empty talks. – Agat Nov 12 '13 at 19:19
  • The user is specifically asking if he should call `AcceptFromQueueAsync` using `Task.Run`, "should I start a new Task for each async call e.g `Task.Run(AcceptFromQueueAsync)`" That is the question. This answer doesn't answer it, at all. Because it doesn't even *try* to answer that question, it would be correct to write a comment indicating that and for it to be downvoted, as it's not answer. On the other hand you seem to have gone and downvoted my answer simply because you felt I downvoted yours, and not because you feel it is incorrect, unhelpful, or in any way lacking. – Servy Nov 12 '13 at 19:26
  • That seems that we also have here not a perfect phychologist... Anyway, let's leave my dark and "cloudy" soul in peace. The user is **specifically** asked on several things. And one of them also was mentioning the sync call of `DoSyncWork` and "**but I want to allow feature implementation on multiple cores**" (related to "**the above implementation**"). I believe that he was thinking of many things, including the calling of `DoSyncWork` in parallel later. You can believe in whatever you want and act accordingly. Btw, Santa doesn't exist. ;) – Agat Nov 12 '13 at 19:50
  • He *specifically* said that he *can't*. "The CPU heavy method is sync: DoSyncWork() Since the second part is CPU heavy I don't want to use parallelism (**actually can't...**)" So he very specifically said that the work *cannot* be parallelized. There is no need to guess or theorize. – Servy Nov 12 '13 at 19:53
  • Yes. I read that. And from my point of view, he said "actually can't" just because he (sounds not silly and) already mentioned, that he has just one core and there is no sense in parallelism for such CPU-heavy task. However, in future, when several cores appear, the situation might change. – Agat Nov 12 '13 at 20:04
  • When someone says they can't parallelize something it pretty much always means the operation inherently cannot be done in parallel; each portion of the operation is dependent on the previous. Regardless, that's still not what the question is about. The question is about how to start the asynchronous work, to which this answer doesn't answer in the least. – Servy Nov 12 '13 at 20:07
  • 1
    @Servy Don't be silly. This is not "random unrelated information". The user is looking for methodology advice, and this qualifies. By your standards, Stephen Cleary's comment should also be downvoted to oblivion. – nmclean Nov 12 '13 at 20:08
  • The OP asked a specific question. This doesn't answer it. Comments, unlike answers, are not there to answer the question. A comment that provides useful information without answering the question is appropriate. An answer that does so is not. Had you posted this information in a comment it would not have been inappropriate. In fact, I encourage you to convert your post to a comment; that's what it should have been in the first place. Stephen's comment is of course fine. Had he posted that as an answer it would be a bad answer. – Servy Nov 12 '13 at 20:10
0

Running the first part asynchronously might make sense, depends, it has Async in the name which suggests it doesn't block anyway in which case there is no needed, but there is definitely no need to run DoSyncWork asynchronously since its just 1 process and you are waiting for it to finish.

Ashigore
  • 4,618
  • 1
  • 19
  • 39
0

I use a pattern similar to this to try to keep the IO and CPU going as much as possible

public async void DoWork()
{
    Task cpuBoundTask = null;
    Task ioBoundTask = null;

    while(true)
    {
         ioBoundTask = DoIOBoundStuff();
         var ioResult = await ioBoundTask;

         if(cpuBoundTask != null)
         {
             await cpuBoundTask;
         }
         cpuBoundTask = DoCpuBoundStuff(ioResult);
    }
 }
Yaur
  • 7,333
  • 1
  • 25
  • 36