andreasbuhr / cppcoro

A library of C++ coroutine abstractions for the coroutines TS
MIT License
364 stars 53 forks source link

What is a correct way to switch away from coroutine? #78

Closed Ivan-Viktorov closed 11 months ago

Ivan-Viktorov commented 11 months ago

It's not documented and I couldn't find a way to suspend current coroutine and give control to another unspecified coroutine from pool. For example I'm waiting for data to become ready some where (ldap C sdk ldap_result() method for example, or poll()). If I perform co_await pool.schedule() current coroutine is enqueued to local LIFO queue and is resumed before any other task.

Example:

#include <iostream>
#include <vector>
#include <cppcoro/task.hpp>
#include <cppcoro/static_thread_pool.hpp>
#include <cppcoro/sync_wait.hpp>
#include <cppcoro/when_all.hpp>

int main() {
        constexpr int tnum = 1;
        cppcoro::static_thread_pool pool{tnum};
        int task_number = 0;

        auto task = [&pool, &task_number]() -> cppcoro::task<> {
                int id = ++task_number;
                std::cout << "Adding task " << id << " to thread pool." << std::endl;
                co_await pool.schedule();
                std::cout << "Task " << id << " resumed." << std::endl;
                for (int i = 0; i < 10; ++i) {
                        std::cout << "Waiting for something to become ready in task " << id << std::endl;
                        co_await pool.schedule();
                }
                std::cout << "Task " << id << " finished." << std::endl;
        };

        std::vector<cppcoro::task<>> tasks;
        for (int i = 0; i < tnum*2; ++i) {
                tasks.emplace_back(task());
        }
        cppcoro::sync_wait(cppcoro::when_all(std::move(tasks)));
}

Desired result: task 1 and task 2 resumed one by one. Got: task 1 is executed first and task 2 executed only after task 1 finished.

Adding task 1 to thread pool.
Adding task 2 to thread pool.
Task 1 resumed.
Waiting for something to become ready in task 1
Waiting for something to become ready in task 1
Waiting for something to become ready in task 1
Waiting for something to become ready in task 1
Waiting for something to become ready in task 1
Waiting for something to become ready in task 1
Waiting for something to become ready in task 1
Waiting for something to become ready in task 1
Waiting for something to become ready in task 1
Waiting for something to become ready in task 1
Task 1 finished.
Task 2 resumed.
Waiting for something to become ready in task 2
Waiting for something to become ready in task 2
Waiting for something to become ready in task 2
Waiting for something to become ready in task 2
Waiting for something to become ready in task 2
Waiting for something to become ready in task 2
Waiting for something to become ready in task 2
Waiting for something to become ready in task 2
Waiting for something to become ready in task 2
Waiting for something to become ready in task 2
Task 2 finished.

If there is no such mechanism, maybe it is possible to add a flag to static_thread_pool.schedule() which will force queueing current coroutine at the end of remote queue? Or add separate schedule() analogue.

andreasbuhr commented 11 months ago

Hi, well, if you really want to have a FIFO behavior, you can go ahead and extend static_thread_pools's API. Maybe create a new public method which first calls "remote_enqueue(operation);" and then "wake_one_thread();".

But from the information you have given, it seems to me that this would be the wrong approach to your problem. Usually you should co_await whatever you are waiting for. I understand you want to wait on ldap_result(). Maybe you can write a coroutine-enabled layer on top of the ldap SDK. Maybe you can create an awaitable co_ldap_result(). Then you could write "co_await co_ldap_result()" which suspends the current coroutine until the ldap result is available.

If you don't want to do this and keep your try-wait-retry approach, maybe you can do something which captures your intent to wait better. Currently, when you don't have anything to do and you want to wait, you express that by calling "co_await pool.schedule();". However, "co_await pool.schedule();" means: "I do have work, go ahead and do it in the thread pool". Maybe, instead, you can use some coroutine-enabled timer where you can say "co_await sleep(100);". This would reflect your intent to wait very good. After the timer, you can use "co_await pool.schedule();" to get back into the thread pool.

Does this make sense to you?

Ivan-Viktorov commented 11 months ago

Hi, thanks for answer.

Do you mean that I can create a pull request with such API or that I should extend it in my user code? The latter is problem due to wake_on_thread() and remote_enqueue() are private, not even protected.

I'm not familiar with coroutines and looks like I'm missing some conceptions, but using existing libraries without big wrappers and additional code looks more preferable to me, at least for initial implementations.

andreasbuhr commented 11 months ago

I'd suggest you fork cppcoro and adapt the fork to your needs. No need for a pull request.