I can not understand why this code is bad:

public ICollection<Product> GetAllProducts() { //getting data from DB } public async Task<ICollection<Product>> GetAllProductsAsync() { return await Task<ICollection<Product>>.Factory.StartNew(GetAllProducts) }
  • What is considered? For the first time I hear about it. - ixSci
  • @ixSci, I, in general, also hear for the first time, but there are no advantages to such wrapping. - Qwertiy ♦
  • @ixSci developers async/await :). - andreycha

4 answers 4

I will not respond on my own, but simply quote the words of .NET developers. After all, who else but they are the ultimate truth about how something should work.

Below is a very brief summary of the post "Should I expose asynchronous wrappers for synchronous methods?" from async / await developers and TAP creators. Know the language - read, do not know - translate by Google. But be sure to read.

Asynchrony

The two main advantages of asynchrony are scalability and unloading of the main thread (responsiveness of UI, concurrency). Different types of applications, as a rule, need different advantages: server applications need scalability, UI applications need unloading.

Scalability

The ability to run synchronous methods as asynchronous does not increase scalability, since the amount of resources spent remains the same — instead of the current thread, it runs in a different stream. Scalability of applications is achieved due to the fact that with the right asynchrony, the same resources do more work.

Unloading

In case of increasing responsiveness of UI and running code in parallel threads, wrapping the synchronous code in asynchronous is what you need.

Discussion

The .NET development team in Task-based Asynchronous Pattern does not recommend holding asynchronous methods in the API that simply wrap synchronous ones.

As already mentioned, in order to improve scalability, the code must be truly asynchronous — that is, source synchronous code needs to be redone. In the case of unloading - the original synchronous code does not need alterations. Moreover, if you provide only a synchronous version to the API, you get some bonuses:

  • Less methods - less headache. Developers need less maintenance and testing. API users are less likely to suffer from the choice of which method to use in each particular case.
  • According to the guideline, all methods contained in the API must be truly asynchronous. Those. Seeing the asynchronous method in the API, the user of the API needs to be sure that using it will improve scalability.
  • The choice of whether to run the synchronous method asynchronously remains with the user. If he can "suffer" / allow circumstances - he will run the method synchronously. If he needs responsiveness, he will run the method asynchronously.

The main conclusion: the API should not lie and mislead .

From myself I can only say that as long as you write programs for yourself, or while you write code that no one else uses, these arguments may seem empty. But believe me, with serious development in the team, and even more so when creating libraries that are used by many developers, these rules turn out to be very useful.

It is also worth adding that the discussion above is about a public API . The degree of publicity may be different - it may be a library that is installed in hundreds with a nuge, or it may be your colleagues who use the component you have developed. It is only important that the public API is not misleading.

The statement "async over sync is bad" does not in any way apply to all code. Because, for example, if in the fully synchronous code they begin to use asynchronous methods, then it is clear that until the application is completely "penetrated" async/await , the code will contain "joints": asynchronous wrappers for synchronous methods. This is normal, this is a workflow. It is important to understand that async over sync should be either a forced measure (in the case of unloading) or a temporary measure (in the case of mixing), but not the norm .

The fact is that any thing (even a language feature, even a household appliance) should be made so that it would be easy to use correctly and difficult to use incorrectly. Unfortunately, the main advantage of async/await - easy asynchronous code writing - is at the same time its disadvantage. For example, because you can easily provide an asynchronous wrapper for synchronous code. This may lead to the fact that the developer can pair up with any synchronous method to provide an asynchronous wrapper. If we recall the previous patterns of asynchronous programming in .NET - Asynchronous Programming Model (methods BeginXXX / EndXXX ) and Event-based Asynchronous Pattern (method XXXAsync and event XXXCompleted ) - then it is much more difficult to make an asynchronous wrapper for the synchronous method. And the next thought after “And let me do the asynchronous version” will be the thought “Oh, well, who needs it, they will do it themselves”. Those. These models made you think carefully about whether an asynchronous wrapper is really needed. When using async/await this barrier is absent, so the developers have to actively talk and remind about this feature.

  • "If he can suffer - he will run the method asynchronously" - is it synchronous? - Qwertiy ♦
  • @Qwertiy yes, thanks. - andreycha
  • The developers of a particular language feature are never “ultimate truth” in design matters. Although I agree with these arguments specifically, there are other cases, such as the old API and other things that you build not from scratch, but for some framework and introduce a rule: artificial asynchronous methods are bad - stupid. Asynchronous programming was born long before async / await and not the MS of all “life to learn,” in this regard. - ixSci 5:46 pm
  • one
    @ixSci updated the answer. And yes, the question is not about asynchronous programming in general , but only about async/await . So what about "never are" I would not be so categorical. - andreycha
  • one
    @ sp7 is a bad example, because this class is just a stub. And at the level of the system (OS) there are no tasks, there is an asynchronous API, which BCL classes use and put tasks outside . Download synchronous execution in a separate thread, of course, it makes sense in some situations. The point is to provide this to the client. And do not provide a supposedly asynchronous method, which in fact is not truly asynchronous. - andreycha

At a minimum, by spending extra resources.

For example, Windows supports asynchronous I / O operations, so to wait for a piece of data, you do not need to create a new stream and send it to hibernation - the system itself will notify you that the reading is complete.

Similarly, it can be with other operations.

If you provide an async version, then it is expected to have a more optimal use of resources than from the usual one. In the end, the calling code itself can start a normal operation via StartNew . And, if he needs 10 such operations, he will run them all over the pack, and not 10 times. And it does not spend on secondary work 10 times more than with the fact that each async-method makes StartNew.

    async / await, depending on the type of application, gives two different bonuses:

    • In Desktop (WPF / Winforms), this mechanism allows you to transfer part of the work to the background thread. The UI thread in this case is a valuable resource, and asynchrony by running the background thread makes sense. But in this case, it is easier to write all the called code synchronously, and make the wrappers at the top.
    • There is no stream in ASP.NET UI. The request processing code is executed on the thread from the pool. Your code frees this thread (assuming that you have an asynchronous controller at the very top). But at the same time eats another thread from the pool to perform the operation. Those. your code as used a stream from a pool, and uses. But now you pay the extra cost of asynchrony.

    In the case of ASP.NET, you may be tempted to launch two calls to the database in one thread (aka "parallelize"). The problem is that in ASP.NET, threads and kernels are a valuable resource. If the server has to load two cores to execute the request, it will be able to process two times less simultaneous requests. Those. on a development machine, you will receive acceleration, but not on a server under load.

    With proper use - pulling async from network or disk operations - async / await will save you streams. If it is wrong, then vice versa, it will spend it.

    Those. There may be cases when wrapping will result in a significant gain, but this should be done very carefully - otherwise you will get the exact opposite of the expected effect.

    • If the server has to load two cores to execute the request, it will be able to process two times less simultaneous requests. Those. on a development machine, you will receive acceleration, but not on a server under load. If the server has enough resources for parallel requests, the client will receive a response faster. Not to be missed, will get about the same. So there is a benefit. - Drag13
    • @Vitalii servers in production should not be idle - i.e. if the server has enough resources for parallel requests, it means that you overpay for iron twice. On my current project, they tried to "parallelize" the execution of the query - and it hit hard on performance. Moreover, the performance of the production before / after really measured, and not just evaluated "by eye". It turned out that “about the same” is too optimistic. - PashaPash ♦
    • @Vitalii threads require synchronization, sometimes they require time to start (if there is no free in the pool), they require memory (from megabytes to a stream). and at the same time they do not solve the problems with the main bottleneck - network calls and other io-bound. Those. "get about the same" - this is a very optimistic estimate. in practice - "will receive guaranteed slower than without parallelization" :) - PashaPash ♦
    • Thank you for the answer and for sharing your experience. It turns out that we can get a win when the system is underutilized (and pay a higher cost) and get a problem when fully loaded when allocating streams will cost us resources that we don’t have. It turns out that piling is meaningful when speed is important to us and we are ready to pay for it, simply by raising the upper level of server capacity. Am I thinking correctly? - Drag13
    • one
      @Vitalii Something like this, but in practice it is worth explicitly measuring the difference for each specific change. It is usually much easier (cheaper) not to waste time on coding, but to take a processor / vm more expensive. For example, on the same azure, you can simply swap virutalki from A to D / D2, and get a performance boost twice :) Parallelizing should be the last step when you really have a CPU-bound code, and other optimization options do not help. - PashaPash ♦
     //getting data from DB

    this comment is most likely to work with a network or file, these operations should be performed asynchronously.

    The author of the code makes an asynchronous wrapper over a synchronous method, which by nature must be asynchronous.

    I admit the only scenario why this happens sometimes: the interface for querying the database is synchronous, and the interface that the author implements requires an asynchronous signature.

    Even if so, it looks more logical

      public Task<ICollection<Product>> GetAllProductsAsync() { return Task.FromResult(GetAllProducts()); }
    • four
      Not more logical. The rule "cannot block a calling thread in an asynchronous method" is stronger than the one discussed. - Pavel Mayorov
    • @PavelMayorov, but could not explain in more detail? And how best to get out of the situation described by the author of the answer? - klutch1991
    • @ klutch1991 there are three answers above you, and one of them is accepted - Pavel Mayorov