search

adamhathcock / sharpcompress

SharpCompress is a fully managed C# library to deal with many compression types and formats.
MIT License
2.26k stars 479 forks source link

Switch to Async IO methods #488

Open adamhathcock opened 4 years ago

adamhathcock commented 4 years ago

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

adamhathcock commented 4 years ago

Need to look at Spans and whatnot. Not sure if I have the time 😢

Need to reimplement everything?

JanHyka commented 4 years ago

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, Jan

adamhathcock commented 4 years ago

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.

JanHyka commented 4 years ago

There is an article about various approaches: https://docs.microsoft.com/en-us/archive/msdn-magazine/2015/july/async-programming-brownfield-async-development

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?

adamhathcock commented 4 years ago

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.

adamhathcock commented 3 years ago

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

adamhathcock commented 3 years ago

Started https://github.com/adamhathcock/sharpcompress/pull/565 but looking for help and comments

datvm commented 1 year ago

My Stream implementation only supports ReadAsync and Read throws. Is there anyway I can help with this?

adamhathcock commented 1 year ago

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 Stream needs to use the async variant and it has to bubble up. That's just for starters :(