Skip to content
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

C++20 coroutine support #1155

Open
simue opened this issue Dec 20, 2024 · 2 comments
Open

C++20 coroutine support #1155

simue opened this issue Dec 20, 2024 · 2 comments

Comments

@simue
Copy link

simue commented Dec 20, 2024

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

@COM8
Copy link
Member

COM8 commented Dec 21, 2024

@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.

@simue
Copy link
Author

simue commented Jan 8, 2025

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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants