add `unifex::v2::async_scope` #463

janondrusek · 2022-09-28T21:03:45Z

simpler than unifex::v1::async_scope (nest() and join())
does not support cancellation

* simpler than `unifex::v1::async_scope` (`nest()` and `join()`) * does not support cancellation Co-authored-by: Ian Petersen <ispeters@gmail.com>

Like in PR #463, this diff disables the static noexcept checks when building with gcc 9 because that compiler seems to get the noexcept calculations wrong.

…perimental#372) * Make async_scope::spawn return a lazy<> This diff changes `unifex::async_scope::spawn(Sender auto)` to return a new type, `unifex::lazy<...>`. `lazy<>` is a _Sender_ that completes with the result of the _Sender_ given to `spawn`. * Fix memory leak * Rationalize the names of some internal bits * Fuse the promise and the operation state This diff merges the spawned operation's promise with its operation state so there's only one allocation. One consequence of this change is that outstanding operations don't record themselves as complete within their corresponding scope until the associated `lazy<>` is either connected and started, or discarded. * Fix build Looks like our CI turns on exhaustive switch warnings, which I broke. * Fix infinite hang in create_test.cpp * Cleanup * Last bit of cleanup before bed * Try to fix ASAN failure In the light of the morning, I think it's wrong to set the event before requesting stop on the stop source because setting the event could lead to destruction of the operation state, invalidating the stop source. * Fix request_stop() The callers of `request_stop()` are not owners so they may not call `decref()`, which means I can't invoke `set_done()` from `request_stop()`. * Clean up, bug fixes, comments Among other clean-up, this diff fixes a stack-use-after-return in `future<>`'s `connect()`. * Add constraints and run clang-format * Comments, tests, bug fixes, and clang-format * Restore nothrow assertion in detached_spawn_call_on * Round out the async_scope tests add async_scope::attach (facebookexperimental#392) * returned `Sender` needs to be connected and started * avoids paying penalty of `future<>` Fix `record_done` ordering in `async_scope::attach` (facebookexperimental#424) * `record_done`, which decrements outstanding operation count, must be called after `set_*` * add regression test fix cancellation race in async_scope::attach* (facebookexperimental#425) * use refcount to pass ownership of `deliver_result` * replace `fused_stop_source` with `inplace_stop_source` make `async_scope::attached_sender` copyable (facebookexperimental#428) * copy constructor calls `async_scope::try_record_start` internally * copy of attached Sender will increment outstanding number of operations on async_scope `async_scope::attach` cleanup (facebookexperimental#433) * remove unused template argument * add missing test case update `async_scope` docs (facebookexperimental#434) * spawn() -> detached_spawn() * spawn() returns a `future` * add missing public methods add `async_scope::attach` docs (facebookexperimental#434) fix `async_scope_test::attach_record_done` (facebookexperimental#437) fix `tag_invoke(CPO)` in `async_scope` (facebookexperimental#462) add `unifex::v2::async_scope` (facebookexperimental#463) * simpler than `unifex::v1::async_scope` (`nest()` and `join()`) * does not support cancellation Introduce unifex::nest() (facebookexperimental#468) `unifex::nest()` is a CPO that delegates to either a `tag_invoke` customization taking a *Sender* and a "scope" reference, or to a member function on the given scope that takes a *Sender*. This diff wires `unifex::nest()` to the `nest()` member function on `v2::async_scope` and to the `attach()` member function on `v1::async_scope`. Introduce spawn_detached(sender, scope, allocator) (facebookexperimental#470) This diff introduces a new algorithm, `unifex::spawn_detached()`. `spawn_detached` takes a sender, an "async scope", and an optional allocator. It nests the sender in the scope with `unifex::nest`, allocates and operation state using the allocator, and starts that operation. The given async scope may be anything that `nest()` supports, which currently includes both `v1::async_scope` and `v2::async_scope`. Add an internal receiver to v2::async_scope's nest op (facebookexperimental#484) While writing `unifex::spawn_future()`, I discovered that waiting until the destructor of the `v2::async_scope`'s `nest()` operation to drop the scope reference is too late (it led to hangs). This diff adds an internal receiver to the nest operation so we can detect when the operation is complete (which is likely before the operation state is destroyed) and drop the reference as soon as we reach that state. Fix stop_when's handling of stop requests (facebookexperimental#500) * Fix stop_when's handling of stop requests When trying to sync PR facebookexperimental#495 into our internal repo, I discovered that there's a lifetime issue in `stop_when()`. If the `stop_when()` operation receives a stop request from its Receiver and the last-to-finish child operation completes synchronously in response to the stop request then `stop_when()`'s stop callback will access the internal stop source after it's been destroyed. This first diff just formats `test/stop_when_test.cpp` and `include/unifex/stop_when.hpp` with `clang-format` in preparation for fixing the above problem. * Add a broken unit test This diff adds a unit test to `test/stop_when_test.cpp` that crashes when ASAN is enabled because it dereferences a destroyed `inplace_stop_source`. * Increment stop_when's refcount in its stop callback This diff fixes the broken test from the previous diff by incrementing the `stop_when` operation's refcount while processing a stop request from the receiver. Add spawn_future() (facebookexperimental#489) This diff adds `unifex::spawn_future(Sender, Scope)`. Implement async_scope::spawn with spawn_future (facebookexperimental#501) This diff reimplements the `v1::async_scope::spawn()` method in terms of the newly-added `unifex::spawn_future()` algorithm. I had to delete a few tests that exercise behaviour that's no longer supported. Work around an MSVC bug in C++20 mode (facebookexperimental#492) * merge unit test that depends on `v2/async_scope` Co-authored-by: Ian Petersen <ispeters@gmail.com>

* Make async_scope::spawn return a lazy<> This diff changes `unifex::async_scope::spawn(Sender auto)` to return a new type, `unifex::lazy<...>`. `lazy<>` is a _Sender_ that completes with the result of the _Sender_ given to `spawn`. * Fix memory leak * Rationalize the names of some internal bits * Fuse the promise and the operation state This diff merges the spawned operation's promise with its operation state so there's only one allocation. One consequence of this change is that outstanding operations don't record themselves as complete within their corresponding scope until the associated `lazy<>` is either connected and started, or discarded. * Fix build Looks like our CI turns on exhaustive switch warnings, which I broke. * Fix infinite hang in create_test.cpp * Cleanup * Last bit of cleanup before bed * Try to fix ASAN failure In the light of the morning, I think it's wrong to set the event before requesting stop on the stop source because setting the event could lead to destruction of the operation state, invalidating the stop source. * Fix request_stop() The callers of `request_stop()` are not owners so they may not call `decref()`, which means I can't invoke `set_done()` from `request_stop()`. * Clean up, bug fixes, comments Among other clean-up, this diff fixes a stack-use-after-return in `future<>`'s `connect()`. * Add constraints and run clang-format * Comments, tests, bug fixes, and clang-format * Restore nothrow assertion in detached_spawn_call_on * Round out the async_scope tests add async_scope::attach (#392) * returned `Sender` needs to be connected and started * avoids paying penalty of `future<>` Fix `record_done` ordering in `async_scope::attach` (#424) * `record_done`, which decrements outstanding operation count, must be called after `set_*` * add regression test fix cancellation race in async_scope::attach* (#425) * use refcount to pass ownership of `deliver_result` * replace `fused_stop_source` with `inplace_stop_source` make `async_scope::attached_sender` copyable (#428) * copy constructor calls `async_scope::try_record_start` internally * copy of attached Sender will increment outstanding number of operations on async_scope `async_scope::attach` cleanup (#433) * remove unused template argument * add missing test case update `async_scope` docs (#434) * spawn() -> detached_spawn() * spawn() returns a `future` * add missing public methods add `async_scope::attach` docs (#434) fix `async_scope_test::attach_record_done` (#437) fix `tag_invoke(CPO)` in `async_scope` (#462) add `unifex::v2::async_scope` (#463) * simpler than `unifex::v1::async_scope` (`nest()` and `join()`) * does not support cancellation Introduce unifex::nest() (#468) `unifex::nest()` is a CPO that delegates to either a `tag_invoke` customization taking a *Sender* and a "scope" reference, or to a member function on the given scope that takes a *Sender*. This diff wires `unifex::nest()` to the `nest()` member function on `v2::async_scope` and to the `attach()` member function on `v1::async_scope`. Introduce spawn_detached(sender, scope, allocator) (#470) This diff introduces a new algorithm, `unifex::spawn_detached()`. `spawn_detached` takes a sender, an "async scope", and an optional allocator. It nests the sender in the scope with `unifex::nest`, allocates and operation state using the allocator, and starts that operation. The given async scope may be anything that `nest()` supports, which currently includes both `v1::async_scope` and `v2::async_scope`. Add an internal receiver to v2::async_scope's nest op (#484) While writing `unifex::spawn_future()`, I discovered that waiting until the destructor of the `v2::async_scope`'s `nest()` operation to drop the scope reference is too late (it led to hangs). This diff adds an internal receiver to the nest operation so we can detect when the operation is complete (which is likely before the operation state is destroyed) and drop the reference as soon as we reach that state. Fix stop_when's handling of stop requests (#500) * Fix stop_when's handling of stop requests When trying to sync PR #495 into our internal repo, I discovered that there's a lifetime issue in `stop_when()`. If the `stop_when()` operation receives a stop request from its Receiver and the last-to-finish child operation completes synchronously in response to the stop request then `stop_when()`'s stop callback will access the internal stop source after it's been destroyed. This first diff just formats `test/stop_when_test.cpp` and `include/unifex/stop_when.hpp` with `clang-format` in preparation for fixing the above problem. * Add a broken unit test This diff adds a unit test to `test/stop_when_test.cpp` that crashes when ASAN is enabled because it dereferences a destroyed `inplace_stop_source`. * Increment stop_when's refcount in its stop callback This diff fixes the broken test from the previous diff by incrementing the `stop_when` operation's refcount while processing a stop request from the receiver. Add spawn_future() (#489) This diff adds `unifex::spawn_future(Sender, Scope)`. Implement async_scope::spawn with spawn_future (#501) This diff reimplements the `v1::async_scope::spawn()` method in terms of the newly-added `unifex::spawn_future()` algorithm. I had to delete a few tests that exercise behaviour that's no longer supported. Work around an MSVC bug in C++20 mode (#492) * merge unit test that depends on `v2/async_scope` Co-authored-by: Ian Petersen <ispeters@gmail.com>

This commit implements scheduler affinity -- aka "sticky" scheduling -- in `unifex::task<>`. The idea is that it is impossible for a child operation to cause the current coroutine to resume on the wrong execution context. * `task<>`-based coroutines track and propagate the current scheduler * `at_coroutine_exit` remembers current scheduler from when the cleanup action is scheduled * `schedule` always returns an instance of `sender_for<schedule, the_sender>`, which is also a `scheduler_provider` * scheduler affinity when co_await-ing senders in a `task<>`-returning coroutine * scheduler affinity when co_await-ing awaitables in a `task<>`-returning coroutine * awaitables and senders that are `blocking_kind::always_inline` don't get a thunk * More senders and awaitables support compile-time blocking queries * `co_await schedule(sched)` is magic in a `task<>`-returning coroutine: it changes execution context and schedules a cleanup action to transition back to the original scheduler Move implementation of special co_await behavior of scheduler senders out of task.hpp Hoist untyped RAII containers for coroutine_handle<> out of task<> and its awaiter (facebookexperimental#329) While looking at the binary size impact of adopting coroutines with `unifex::task<>`, I noticed that a number of operations on `coroutine_handle<T>` are expressed in `unifex::task<>` as if they depend on `T` when they don't. The consequence is extra code. This diff creates a `coro_holder` class that uniquely owns a `coroutine_handle<>` and makes `unifex::task<>` inherit from it. We technically lose some type safety, but it's still correct by construction. This change saves about 1.5 kilobytes in one of our apps. Similar to the above, I noticed binary duplication due to false template parameter dependencies in `unifex::task<>`'s awaiter type. This diff hoists a non-type-specific RAII container for a `coroutine_handle<>` that stores the handle as a `std::uintptr_t` so that `task`'s awaiter can use the low bit as a dirty flag. This change saves another ~1.5 kilobytes in one of our apps. Fix scheduler affinity (facebookexperimental#405) * Fix scheduler affinity We have been storing a `task<>`'s scheduler as an `any_scheduler_ref`, which has proven to be a source of use-after-free bugs. This change switches all the `any_scheduler_ref`s to `any_scheduler`s, fixing the lifetime issues. Make task<>'s thunk-on-resume unstoppable (facebookexperimental#495) * Make task<>'s thunk-on-resume unstoppable When awaiting an async Sender that swallows done signals (such as let_done(never_sender{}, just)), the user-level code looks like it swallows done signals: ``` // never cancels co_await let_done(never_sender{}, just); ``` However, `task<>`'s Scheduler affinity implementation transforms the above code into this: ``` co_await typed_via(let_done(never_sender{}, just), <current scheduler>); ``` The `schedule()` operation inside the injected `typed_via` can emit done if the current stop token has had stop requested, leading to very non-obvious cancellation behaviour that can't be worked around. This diff introduces a pair of regression tests that capture the above scenario, and the analogous scenario of awaiting an async Awaitable that completes with done. The next diff will fix these failing tests. * Change task<>'s thunk-on-resume to be unstoppable This diff fixes the broken tests in the previous diff. Respect blocking_kind in `let_value()` (facebookexperimental#381) * `let_value()` would always assume `blocking_kind::maybe`, which results in potentially unnecessary reschedule on resumption * replicate `blocking_kind` customization from `finally()` fix `variant_sender` blocking kind add `unifex::v2::async_scope` (facebookexperimental#463) * simpler than `unifex::v1::async_scope` (`nest()` and `join()`) * does not support cancellation fixing linter error move deduction guide to namespace scope for gcc-10 in scheduler concept, check copy_constructability after requiring call to schedule() work around gcc-10 bugs Co-authored-by: Eric Niebler <eniebler@boost.org> Co-authored-by: Ian Petersen <ispeters@gmail.com>

This commit implements scheduler affinity -- aka "sticky" scheduling -- in `unifex::task<>`. The idea is that it is impossible for a child operation to cause the current coroutine to resume on the wrong execution context. * `task<>`-based coroutines track and propagate the current scheduler * `at_coroutine_exit` remembers current scheduler from when the cleanup action is scheduled * `schedule` always returns an instance of `sender_for<schedule, the_sender>`, which is also a `scheduler_provider` * scheduler affinity when co_await-ing senders in a `task<>`-returning coroutine * scheduler affinity when co_await-ing awaitables in a `task<>`-returning coroutine * awaitables and senders that are `blocking_kind::always_inline` don't get a thunk * More senders and awaitables support compile-time blocking queries * `co_await schedule(sched)` is magic in a `task<>`-returning coroutine: it changes execution context and schedules a cleanup action to transition back to the original scheduler Move implementation of special co_await behavior of scheduler senders out of task.hpp Hoist untyped RAII containers for coroutine_handle<> out of task<> and its awaiter (facebookexperimental#329) While looking at the binary size impact of adopting coroutines with `unifex::task<>`, I noticed that a number of operations on `coroutine_handle<T>` are expressed in `unifex::task<>` as if they depend on `T` when they don't. The consequence is extra code. This diff creates a `coro_holder` class that uniquely owns a `coroutine_handle<>` and makes `unifex::task<>` inherit from it. We technically lose some type safety, but it's still correct by construction. This change saves about 1.5 kilobytes in one of our apps. Similar to the above, I noticed binary duplication due to false template parameter dependencies in `unifex::task<>`'s awaiter type. This diff hoists a non-type-specific RAII container for a `coroutine_handle<>` that stores the handle as a `std::uintptr_t` so that `task`'s awaiter can use the low bit as a dirty flag. This change saves another ~1.5 kilobytes in one of our apps. Fix scheduler affinity (facebookexperimental#405) * Fix scheduler affinity We have been storing a `task<>`'s scheduler as an `any_scheduler_ref`, which has proven to be a source of use-after-free bugs. This change switches all the `any_scheduler_ref`s to `any_scheduler`s, fixing the lifetime issues. Make task<>'s thunk-on-resume unstoppable (facebookexperimental#495) * Make task<>'s thunk-on-resume unstoppable When awaiting an async Sender that swallows done signals (such as let_done(never_sender{}, just)), the user-level code looks like it swallows done signals: ``` // never cancels co_await let_done(never_sender{}, just); ``` However, `task<>`'s Scheduler affinity implementation transforms the above code into this: ``` co_await typed_via(let_done(never_sender{}, just), <current scheduler>); ``` The `schedule()` operation inside the injected `typed_via` can emit done if the current stop token has had stop requested, leading to very non-obvious cancellation behaviour that can't be worked around. This diff introduces a pair of regression tests that capture the above scenario, and the analogous scenario of awaiting an async Awaitable that completes with done. The next diff will fix these failing tests. * Change task<>'s thunk-on-resume to be unstoppable This diff fixes the broken tests in the previous diff. Respect blocking_kind in `let_value()` (facebookexperimental#381) * `let_value()` would always assume `blocking_kind::maybe`, which results in potentially unnecessary reschedule on resumption * replicate `blocking_kind` customization from `finally()` fix `variant_sender` blocking kind (facebookexperimental#474) add `unifex::v2::async_scope` (facebookexperimental#463) * simpler than `unifex::v1::async_scope` (`nest()` and `join()`) * does not support cancellation fixing linter error (facebookexperimental#414) move deduction guide to namespace scope for gcc-10 in scheduler concept, check copy_constructability after requiring call to schedule() work around gcc-10 bugs Co-authored-by: Eric Niebler <eniebler@boost.org> Co-authored-by: Ian Petersen <ispeters@gmail.com> Co-authored-by: Ondrej Lehecka <lehecka@fb.com>

This commit implements scheduler affinity -- aka "sticky" scheduling -- in `unifex::task<>`. The idea is that it is impossible for a child operation to cause the current coroutine to resume on the wrong execution context. * `task<>`-based coroutines track and propagate the current scheduler * `at_coroutine_exit` remembers current scheduler from when the cleanup action is scheduled * `schedule` always returns an instance of `sender_for<schedule, the_sender>`, which is also a `scheduler_provider` * scheduler affinity when co_await-ing senders in a `task<>`-returning coroutine * scheduler affinity when co_await-ing awaitables in a `task<>`-returning coroutine * awaitables and senders that are `blocking_kind::always_inline` don't get a thunk * More senders and awaitables support compile-time blocking queries * `co_await schedule(sched)` is magic in a `task<>`-returning coroutine: it changes execution context and schedules a cleanup action to transition back to the original scheduler Move implementation of special co_await behavior of scheduler senders out of task.hpp Hoist untyped RAII containers for coroutine_handle<> out of task<> and its awaiter (facebookexperimental#329) While looking at the binary size impact of adopting coroutines with `unifex::task<>`, I noticed that a number of operations on `coroutine_handle<T>` are expressed in `unifex::task<>` as if they depend on `T` when they don't. The consequence is extra code. This diff creates a `coro_holder` class that uniquely owns a `coroutine_handle<>` and makes `unifex::task<>` inherit from it. We technically lose some type safety, but it's still correct by construction. This change saves about 1.5 kilobytes in one of our apps. Similar to the above, I noticed binary duplication due to false template parameter dependencies in `unifex::task<>`'s awaiter type. This diff hoists a non-type-specific RAII container for a `coroutine_handle<>` that stores the handle as a `std::uintptr_t` so that `task`'s awaiter can use the low bit as a dirty flag. This change saves another ~1.5 kilobytes in one of our apps. Fix scheduler affinity (facebookexperimental#405) * Fix scheduler affinity We have been storing a `task<>`'s scheduler as an `any_scheduler_ref`, which has proven to be a source of use-after-free bugs. This change switches all the `any_scheduler_ref`s to `any_scheduler`s, fixing the lifetime issues. Make task<>'s thunk-on-resume unstoppable (facebookexperimental#495) * Make task<>'s thunk-on-resume unstoppable When awaiting an async Sender that swallows done signals (such as let_done(never_sender{}, just)), the user-level code looks like it swallows done signals: ``` // never cancels co_await let_done(never_sender{}, just); ``` However, `task<>`'s Scheduler affinity implementation transforms the above code into this: ``` co_await typed_via(let_done(never_sender{}, just), <current scheduler>); ``` The `schedule()` operation inside the injected `typed_via` can emit done if the current stop token has had stop requested, leading to very non-obvious cancellation behaviour that can't be worked around. This diff introduces a pair of regression tests that capture the above scenario, and the analogous scenario of awaiting an async Awaitable that completes with done. The next diff will fix these failing tests. * Change task<>'s thunk-on-resume to be unstoppable This diff fixes the broken tests in the previous diff. Respect blocking_kind in `let_value()` (facebookexperimental#381) * `let_value()` would always assume `blocking_kind::maybe`, which results in potentially unnecessary reschedule on resumption * replicate `blocking_kind` customization from `finally()` fix `variant_sender` blocking kind (facebookexperimental#474) add `unifex::v2::async_scope` (facebookexperimental#463) * simpler than `unifex::v1::async_scope` (`nest()` and `join()`) * does not support cancellation fixing linter error (facebookexperimental#414) move deduction guide to namespace scope for gcc-10 in scheduler concept, check copy_constructability after requiring call to schedule() work around gcc-10 bugs avoid warning about missing braces in initializer Co-authored-by: Eric Niebler <eniebler@boost.org> Co-authored-by: Ian Petersen <ispeters@gmail.com> Co-authored-by: Ondrej Lehecka <lehecka@fb.com>

This commit implements scheduler affinity -- aka "sticky" scheduling -- in `unifex::task<>`. The idea is that it is impossible for a child operation to cause the current coroutine to resume on the wrong execution context. * `task<>`-based coroutines track and propagate the current scheduler * `at_coroutine_exit` remembers current scheduler from when the cleanup action is scheduled * `schedule` always returns an instance of `sender_for<schedule, the_sender>`, which is also a `scheduler_provider` * scheduler affinity when co_await-ing senders in a `task<>`-returning coroutine * scheduler affinity when co_await-ing awaitables in a `task<>`-returning coroutine * awaitables and senders that are `blocking_kind::always_inline` don't get a thunk * More senders and awaitables support compile-time blocking queries * `co_await schedule(sched)` is magic in a `task<>`-returning coroutine: it changes execution context and schedules a cleanup action to transition back to the original scheduler Move implementation of special co_await behavior of scheduler senders out of task.hpp Hoist untyped RAII containers for coroutine_handle<> out of task<> and its awaiter (facebookexperimental#329) While looking at the binary size impact of adopting coroutines with `unifex::task<>`, I noticed that a number of operations on `coroutine_handle<T>` are expressed in `unifex::task<>` as if they depend on `T` when they don't. The consequence is extra code. This diff creates a `coro_holder` class that uniquely owns a `coroutine_handle<>` and makes `unifex::task<>` inherit from it. We technically lose some type safety, but it's still correct by construction. This change saves about 1.5 kilobytes in one of our apps. Similar to the above, I noticed binary duplication due to false template parameter dependencies in `unifex::task<>`'s awaiter type. This diff hoists a non-type-specific RAII container for a `coroutine_handle<>` that stores the handle as a `std::uintptr_t` so that `task`'s awaiter can use the low bit as a dirty flag. This change saves another ~1.5 kilobytes in one of our apps. Fix scheduler affinity (facebookexperimental#405) * Fix scheduler affinity We have been storing a `task<>`'s scheduler as an `any_scheduler_ref`, which has proven to be a source of use-after-free bugs. This change switches all the `any_scheduler_ref`s to `any_scheduler`s, fixing the lifetime issues. Make task<>'s thunk-on-resume unstoppable (facebookexperimental#495) * Make task<>'s thunk-on-resume unstoppable When awaiting an async Sender that swallows done signals (such as let_done(never_sender{}, just)), the user-level code looks like it swallows done signals: ``` // never cancels co_await let_done(never_sender{}, just); ``` However, `task<>`'s Scheduler affinity implementation transforms the above code into this: ``` co_await typed_via(let_done(never_sender{}, just), <current scheduler>); ``` The `schedule()` operation inside the injected `typed_via` can emit done if the current stop token has had stop requested, leading to very non-obvious cancellation behaviour that can't be worked around. This diff introduces a pair of regression tests that capture the above scenario, and the analogous scenario of awaiting an async Awaitable that completes with done. The next diff will fix these failing tests. * Change task<>'s thunk-on-resume to be unstoppable This diff fixes the broken tests in the previous diff. Respect blocking_kind in `let_value()` (facebookexperimental#381) * `let_value()` would always assume `blocking_kind::maybe`, which results in potentially unnecessary reschedule on resumption * replicate `blocking_kind` customization from `finally()` fix `variant_sender` blocking kind (facebookexperimental#474) add `unifex::v2::async_scope` (facebookexperimental#463) * simpler than `unifex::v1::async_scope` (`nest()` and `join()`) * does not support cancellation fixing linter error (facebookexperimental#414) move deduction guide to namespace scope for gcc-10 in scheduler concept, check copy_constructability after requiring call to schedule() work around gcc-10 bugs avoid warning about missing braces in initializer back out change to awaiter_type_t Co-authored-by: Eric Niebler <eniebler@boost.org> Co-authored-by: Ian Petersen <ispeters@gmail.com> Co-authored-by: Ondrej Lehecka <lehecka@fb.com>

This commit implements scheduler affinity -- aka "sticky" scheduling -- in `unifex::task<>`. The idea is that it is impossible for a child operation to cause the current coroutine to resume on the wrong execution context. * `task<>`-based coroutines track and propagate the current scheduler * `at_coroutine_exit` remembers current scheduler from when the cleanup action is scheduled * `schedule` always returns an instance of `sender_for<schedule, the_sender>`, which is also a `scheduler_provider` * scheduler affinity when co_await-ing senders in a `task<>`-returning coroutine * scheduler affinity when co_await-ing awaitables in a `task<>`-returning coroutine * awaitables and senders that are `blocking_kind::always_inline` don't get a thunk * More senders and awaitables support compile-time blocking queries * `co_await schedule(sched)` is magic in a `task<>`-returning coroutine: it changes execution context and schedules a cleanup action to transition back to the original scheduler Move implementation of special co_await behavior of scheduler senders out of task.hpp Hoist untyped RAII containers for coroutine_handle<> out of task<> and its awaiter (#329) While looking at the binary size impact of adopting coroutines with `unifex::task<>`, I noticed that a number of operations on `coroutine_handle<T>` are expressed in `unifex::task<>` as if they depend on `T` when they don't. The consequence is extra code. This diff creates a `coro_holder` class that uniquely owns a `coroutine_handle<>` and makes `unifex::task<>` inherit from it. We technically lose some type safety, but it's still correct by construction. This change saves about 1.5 kilobytes in one of our apps. Similar to the above, I noticed binary duplication due to false template parameter dependencies in `unifex::task<>`'s awaiter type. This diff hoists a non-type-specific RAII container for a `coroutine_handle<>` that stores the handle as a `std::uintptr_t` so that `task`'s awaiter can use the low bit as a dirty flag. This change saves another ~1.5 kilobytes in one of our apps. Fix scheduler affinity (#405) * Fix scheduler affinity We have been storing a `task<>`'s scheduler as an `any_scheduler_ref`, which has proven to be a source of use-after-free bugs. This change switches all the `any_scheduler_ref`s to `any_scheduler`s, fixing the lifetime issues. Make task<>'s thunk-on-resume unstoppable (#495) * Make task<>'s thunk-on-resume unstoppable When awaiting an async Sender that swallows done signals (such as let_done(never_sender{}, just)), the user-level code looks like it swallows done signals: ``` // never cancels co_await let_done(never_sender{}, just); ``` However, `task<>`'s Scheduler affinity implementation transforms the above code into this: ``` co_await typed_via(let_done(never_sender{}, just), <current scheduler>); ``` The `schedule()` operation inside the injected `typed_via` can emit done if the current stop token has had stop requested, leading to very non-obvious cancellation behaviour that can't be worked around. This diff introduces a pair of regression tests that capture the above scenario, and the analogous scenario of awaiting an async Awaitable that completes with done. The next diff will fix these failing tests. * Change task<>'s thunk-on-resume to be unstoppable This diff fixes the broken tests in the previous diff. Respect blocking_kind in `let_value()` (#381) * `let_value()` would always assume `blocking_kind::maybe`, which results in potentially unnecessary reschedule on resumption * replicate `blocking_kind` customization from `finally()` fix `variant_sender` blocking kind (#474) add `unifex::v2::async_scope` (#463) * simpler than `unifex::v1::async_scope` (`nest()` and `join()`) * does not support cancellation fixing linter error (#414) move deduction guide to namespace scope for gcc-10 in scheduler concept, check copy_constructability after requiring call to schedule() work around gcc-10 bugs avoid warning about missing braces in initializer back out change to awaiter_type_t Co-authored-by: Eric Niebler <eniebler@boost.org> Co-authored-by: Ian Petersen <ispeters@gmail.com> Co-authored-by: Ondrej Lehecka <lehecka@fb.com>

…perimental#372) * Make async_scope::spawn return a lazy<> This diff changes `unifex::async_scope::spawn(Sender auto)` to return a new type, `unifex::lazy<...>`. `lazy<>` is a _Sender_ that completes with the result of the _Sender_ given to `spawn`. * Fix memory leak * Rationalize the names of some internal bits * Fuse the promise and the operation state This diff merges the spawned operation's promise with its operation state so there's only one allocation. One consequence of this change is that outstanding operations don't record themselves as complete within their corresponding scope until the associated `lazy<>` is either connected and started, or discarded. * Fix build Looks like our CI turns on exhaustive switch warnings, which I broke. * Fix infinite hang in create_test.cpp * Cleanup * Last bit of cleanup before bed * Try to fix ASAN failure In the light of the morning, I think it's wrong to set the event before requesting stop on the stop source because setting the event could lead to destruction of the operation state, invalidating the stop source. * Fix request_stop() The callers of `request_stop()` are not owners so they may not call `decref()`, which means I can't invoke `set_done()` from `request_stop()`. * Clean up, bug fixes, comments Among other clean-up, this diff fixes a stack-use-after-return in `future<>`'s `connect()`. * Add constraints and run clang-format * Comments, tests, bug fixes, and clang-format * Restore nothrow assertion in detached_spawn_call_on * Round out the async_scope tests add async_scope::attach (facebookexperimental#392) * returned `Sender` needs to be connected and started * avoids paying penalty of `future<>` Fix `record_done` ordering in `async_scope::attach` (facebookexperimental#424) * `record_done`, which decrements outstanding operation count, must be called after `set_*` * add regression test fix cancellation race in async_scope::attach* (facebookexperimental#425) * use refcount to pass ownership of `deliver_result` * replace `fused_stop_source` with `inplace_stop_source` make `async_scope::attached_sender` copyable (facebookexperimental#428) * copy constructor calls `async_scope::try_record_start` internally * copy of attached Sender will increment outstanding number of operations on async_scope `async_scope::attach` cleanup (facebookexperimental#433) * remove unused template argument * add missing test case update `async_scope` docs (facebookexperimental#434) * spawn() -> detached_spawn() * spawn() returns a `future` * add missing public methods add `async_scope::attach` docs (facebookexperimental#434) fix `async_scope_test::attach_record_done` (facebookexperimental#437) fix `tag_invoke(CPO)` in `async_scope` (facebookexperimental#462) add `unifex::v2::async_scope` (facebookexperimental#463) * simpler than `unifex::v1::async_scope` (`nest()` and `join()`) * does not support cancellation Introduce unifex::nest() (facebookexperimental#468) `unifex::nest()` is a CPO that delegates to either a `tag_invoke` customization taking a *Sender* and a "scope" reference, or to a member function on the given scope that takes a *Sender*. This diff wires `unifex::nest()` to the `nest()` member function on `v2::async_scope` and to the `attach()` member function on `v1::async_scope`. Introduce spawn_detached(sender, scope, allocator) (facebookexperimental#470) This diff introduces a new algorithm, `unifex::spawn_detached()`. `spawn_detached` takes a sender, an "async scope", and an optional allocator. It nests the sender in the scope with `unifex::nest`, allocates and operation state using the allocator, and starts that operation. The given async scope may be anything that `nest()` supports, which currently includes both `v1::async_scope` and `v2::async_scope`. Add an internal receiver to v2::async_scope's nest op (facebookexperimental#484) While writing `unifex::spawn_future()`, I discovered that waiting until the destructor of the `v2::async_scope`'s `nest()` operation to drop the scope reference is too late (it led to hangs). This diff adds an internal receiver to the nest operation so we can detect when the operation is complete (which is likely before the operation state is destroyed) and drop the reference as soon as we reach that state. Fix stop_when's handling of stop requests (facebookexperimental#500) * Fix stop_when's handling of stop requests When trying to sync PR facebookexperimental#495 into our internal repo, I discovered that there's a lifetime issue in `stop_when()`. If the `stop_when()` operation receives a stop request from its Receiver and the last-to-finish child operation completes synchronously in response to the stop request then `stop_when()`'s stop callback will access the internal stop source after it's been destroyed. This first diff just formats `test/stop_when_test.cpp` and `include/unifex/stop_when.hpp` with `clang-format` in preparation for fixing the above problem. * Add a broken unit test This diff adds a unit test to `test/stop_when_test.cpp` that crashes when ASAN is enabled because it dereferences a destroyed `inplace_stop_source`. * Increment stop_when's refcount in its stop callback This diff fixes the broken test from the previous diff by incrementing the `stop_when` operation's refcount while processing a stop request from the receiver. Add spawn_future() (facebookexperimental#489) This diff adds `unifex::spawn_future(Sender, Scope)`. Implement async_scope::spawn with spawn_future (facebookexperimental#501) This diff reimplements the `v1::async_scope::spawn()` method in terms of the newly-added `unifex::spawn_future()` algorithm. I had to delete a few tests that exercise behaviour that's no longer supported. Work around an MSVC bug in C++20 mode (facebookexperimental#492) * merge unit test that depends on `v2/async_scope` Co-authored-by: Ian Petersen <ispeters@gmail.com>

This commit implements scheduler affinity -- aka "sticky" scheduling -- in `unifex::task<>`. The idea is that it is impossible for a child operation to cause the current coroutine to resume on the wrong execution context. * `task<>`-based coroutines track and propagate the current scheduler * `at_coroutine_exit` remembers current scheduler from when the cleanup action is scheduled * `schedule` always returns an instance of `sender_for<schedule, the_sender>`, which is also a `scheduler_provider` * scheduler affinity when co_await-ing senders in a `task<>`-returning coroutine * scheduler affinity when co_await-ing awaitables in a `task<>`-returning coroutine * awaitables and senders that are `blocking_kind::always_inline` don't get a thunk * More senders and awaitables support compile-time blocking queries * `co_await schedule(sched)` is magic in a `task<>`-returning coroutine: it changes execution context and schedules a cleanup action to transition back to the original scheduler Move implementation of special co_await behavior of scheduler senders out of task.hpp Hoist untyped RAII containers for coroutine_handle<> out of task<> and its awaiter (facebookexperimental#329) While looking at the binary size impact of adopting coroutines with `unifex::task<>`, I noticed that a number of operations on `coroutine_handle<T>` are expressed in `unifex::task<>` as if they depend on `T` when they don't. The consequence is extra code. This diff creates a `coro_holder` class that uniquely owns a `coroutine_handle<>` and makes `unifex::task<>` inherit from it. We technically lose some type safety, but it's still correct by construction. This change saves about 1.5 kilobytes in one of our apps. Similar to the above, I noticed binary duplication due to false template parameter dependencies in `unifex::task<>`'s awaiter type. This diff hoists a non-type-specific RAII container for a `coroutine_handle<>` that stores the handle as a `std::uintptr_t` so that `task`'s awaiter can use the low bit as a dirty flag. This change saves another ~1.5 kilobytes in one of our apps. Fix scheduler affinity (facebookexperimental#405) * Fix scheduler affinity We have been storing a `task<>`'s scheduler as an `any_scheduler_ref`, which has proven to be a source of use-after-free bugs. This change switches all the `any_scheduler_ref`s to `any_scheduler`s, fixing the lifetime issues. Make task<>'s thunk-on-resume unstoppable (facebookexperimental#495) * Make task<>'s thunk-on-resume unstoppable When awaiting an async Sender that swallows done signals (such as let_done(never_sender{}, just)), the user-level code looks like it swallows done signals: ``` // never cancels co_await let_done(never_sender{}, just); ``` However, `task<>`'s Scheduler affinity implementation transforms the above code into this: ``` co_await typed_via(let_done(never_sender{}, just), <current scheduler>); ``` The `schedule()` operation inside the injected `typed_via` can emit done if the current stop token has had stop requested, leading to very non-obvious cancellation behaviour that can't be worked around. This diff introduces a pair of regression tests that capture the above scenario, and the analogous scenario of awaiting an async Awaitable that completes with done. The next diff will fix these failing tests. * Change task<>'s thunk-on-resume to be unstoppable This diff fixes the broken tests in the previous diff. Respect blocking_kind in `let_value()` (facebookexperimental#381) * `let_value()` would always assume `blocking_kind::maybe`, which results in potentially unnecessary reschedule on resumption * replicate `blocking_kind` customization from `finally()` fix `variant_sender` blocking kind (facebookexperimental#474) add `unifex::v2::async_scope` (facebookexperimental#463) * simpler than `unifex::v1::async_scope` (`nest()` and `join()`) * does not support cancellation fixing linter error (facebookexperimental#414) move deduction guide to namespace scope for gcc-10 in scheduler concept, check copy_constructability after requiring call to schedule() work around gcc-10 bugs avoid warning about missing braces in initializer back out change to awaiter_type_t Co-authored-by: Eric Niebler <eniebler@boost.org> Co-authored-by: Ian Petersen <ispeters@gmail.com> Co-authored-by: Ondrej Lehecka <lehecka@fb.com>

facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Sep 28, 2022

janondrusek force-pushed the broken-stdlib branch 4 times, most recently from 2856398 to 934284e Compare September 29, 2022 14:44

add unifex::v2::async_scope

cfbb666

* simpler than `unifex::v1::async_scope` (`nest()` and `join()`) * does not support cancellation Co-authored-by: Ian Petersen <ispeters@gmail.com>

janondrusek force-pushed the broken-stdlib branch from b7771ec to cfbb666 Compare September 29, 2022 21:51

jesswong merged commit cd5a019 into facebookexperimental:broken-stdlib Oct 3, 2022

ispeters added a commit that referenced this pull request Oct 12, 2022

Fix gcc 9 build

6c50bb7

Like in PR #463, this diff disables the static noexcept checks when building with gcc 9 because that compiler seems to get the noexcept calculations wrong.

ispeters added a commit that referenced this pull request Oct 12, 2022

Fix gcc 9 build

3248979

Like in PR #463, this diff disables the static noexcept checks when building with gcc 9 because that compiler seems to get the noexcept calculations wrong.

janondrusek mentioned this pull request Apr 25, 2023

Make unifex::async_scope::spawn return a unifex::future<> (#372) #504

Merged

janondrusek mentioned this pull request Apr 28, 2023

task<> scheduler affinity (#290) #506

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add `unifex::v2::async_scope` #463

add `unifex::v2::async_scope` #463

janondrusek commented Sep 28, 2022

add unifex::v2::async_scope #463

add unifex::v2::async_scope #463

Conversation

janondrusek commented Sep 28, 2022

add `unifex::v2::async_scope` #463

add `unifex::v2::async_scope` #463