cpr icon indicating copy to clipboard operation
cpr copied to clipboard

C++20 coroutine support

Open simue opened this issue 1 year ago • 5 comments

Is your feature request related to a problem?

Compared with async callback chains, coroutines look like they offer a natural way to write transactions consisting of several chained http requests. Compared with implementing transactions sequentially and achieving parallelism by one thread per transaction, coroutines would save ressources.

In #382 coroutines were hinted, but did not make it into the final implementation. So essentially I just have one question: Is this still being considered? If not: what were the reasons that speak against coroutines in libcpr?

I am just getting into coroutines in C++, so if there are technical reasons that speak against adding coroutines to libcpr, please let me know.

Possible Solution

Having awaitable http requests to achieve something along the lines of:

MyCoroutine SomeImportantTransaction(cpr::Session session) {
  co_await session.Get(...) // get token
  // ..error handling etc.
  co_await session.Post(...) // upload data
  // ..error handling etc.
  co_await session.Get(...) // read result data
  // ..error handling
}

honestly I have no clear picture of coroutines, yet. So the idea is to have awaitable functions for http requests that are then easily chainable in a clear and concise way.

Alternatives

One could alternatively use threads with sequentially implemented transactions, or chaining async callbacks with e.g. cpr's GetCallback()

Both have downsides described in my initial statement

Additional Context

No response

simue avatar Dec 20 '24 17:12 simue

@simue thanks for your request!

Yes, coroutine are still a feature I consider would be awesome for cpr and I would love to have them supported. But it boils down to me not having enough time to implement it. Right now cpr targets c++17 and coroutines would require special feature flags - which is absolutely no deal braker just requires a bit of additional work.

I focus right now on fixing existing bugs compared to adding huge new features.

But I'm always welcoming PRs that add such functionality.

COM8 avatar Dec 21 '24 12:12 COM8

Thanks for the clarification, sounds good! In principle I'd be open to give it a try.

However, I am now doubting whether this is a good idea, actually. Currently async requests seem to be implemented such that every request is executed in its own thread. The thread is blocking for the response (Session::DoEasyPerform). That would mean that additionally to having a thread pool with threads that are blocked until a response is available, coroutines would add one more layer of abstraction with heap allocated stack frames and their management.

What are your thoughts on this?

simue avatar Jan 08 '25 14:01 simue

Yes. You are right. In the best case we could merge both. The thread pool is really buggy right now (example: https://github.com/libcpr/cpr/issues/1040) and needs a rewrite anyway.

COM8 avatar Jan 10 '25 10:01 COM8

Sorry, but I think this is more effort than what I can invest here currently.

simue avatar Feb 03 '25 22:02 simue

OK, no problem! Thanks for reporting back to me!

COM8 avatar Feb 08 '25 07:02 COM8