-
Notifications
You must be signed in to change notification settings - Fork 480
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Switch to Async IO methods #488
Comments
Need to look at Spans and whatnot. Not sure if I have the time 😢 Need to reimplement everything? |
Hello Adam, I would pick this up if possible as we have a need for async approach for our project. What approach would you prefer? Retiring sync entirely (and thus hitting compatibility issues for current consumers) or providing both sync and async methods side by side? For sake of compatibility, I'd go option 2. Perhaps along with marking sync methods as deprecated, if you wish so. Thanks, |
I'm actually less concerned about breaking changes but my understanding is that there is built-in support for async by default but still can use sync? I'm willing to accept just about anything done in this area. I just don't have the personal time to devote to big changes. |
There is an article about various approaches: To me, the most reasonable approach atm seems be to go with 'Flag Argument Hack' approach so there is as little messing and no duplication in the business logic itself. WDYT? |
That could work. I'm still unsure how much async will help because the code itself is CPU bound....but directly linking to network (or large files locally) would make it help. I guess, in general, I was hoping just reimplementing the Stream classes would be good enough to cover sync/async and the rest of the code is all Async. |
Defintely going to have to move everything to async then the blocking versions won't work. Not sure if I just should break the entire API or try to have a blocking hack in place to break less users |
Started #565 but looking for help and comments |
My Stream implementation only supports |
Please do but you'll probably have to start from scratch. It's a lot of grunt work to get right unfortunately. Each instance of |
Need to use async IO now. Guidance for ASP.NET spells it out: https://docs.microsoft.com/en-us/aspnet/core/performance/performance-best-practices
Look to where ValueTask can be used
The text was updated successfully, but these errors were encountered: