NVIDIA / NVIDIA/stdexec

continues_on: hop opstate not connected with secondary environment

Open
#2,268 3 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
C++
Stars
2.4k
Forks
270
Avg merge
3d 6h
Merged PRs (30d)
39

Description

Recent work has introduced the get_start_scheduler query.

Certain algorithms internally start an operation state from the completion of the predecessor. This is the case for instance for let_value, which correspondingly passes a secondary environment with the start scheduler set accordingly.

The continues_on algorithm is also an algorithm that starts an operation state from the completion of the predecessor, in this case to make the hop to the target scheduler:

https://github.com/NVIDIA/stdexec/blob/5f94dbac91de3c4869fe695b7fe4d0ed66c0612d/include/stdexec/__detail/__continues_on.hpp#L127

https://github.com/NVIDIA/stdexec/blob/5f94dbac91de3c4869fe695b7fe4d0ed66c0612d/include/stdexec/__detail/__continues_on.hpp#L385

However, in the case of continues_on, such a secondary environment is not passed:

https://github.com/NVIDIA/stdexec/blob/5f94dbac91de3c4869fe695b7fe4d0ed66c0612d/include/stdexec/__detail/__continues_on.hpp#L193

Because of this issue, compile-time queries such as get_start_scheduler and get_completion_scheduler may give the wrong answer. Indeed, in the hop opstate, the schedule sender is connected to an environment that indicates that the start scheduler is the start scheduler of the outer receiver, rather than the completion scheduler of the predecessor. A striking consequence is that if we hop onto an inline_scheduler, the query wrongly indicates that the completion scheduler is the start scheduler of the outer receiver.

A possible solution is probably to pass a secondary environment to the hop opstate along the lines of what has been done for let_value and recently for starts_on. I.e., on the quoted line 193, we probably need to pass the attributes of the predecessor through the secondary environment pattern; with a corresponding fix where the schedule sender is connected.

Tagging @ericniebler and @Cra3z as this seems related to your recent work on the start scheduler, the completion scheduler, and starts_on.

reproducer
#define THEN_STORE_THREAD_ID(p) stdexec::then([p] { *(p) = std::this_thread::get_id(); })

consteval bool test_is_forwarding_query() {
    return stdexec::forwarding_query(stdexec::get_start_scheduler);
}
static_assert(test_is_forwarding_query());

/**
 * @test Check that the start scheduler that @c stdexec::sync_wait sets in the receiver environment
 *       is a @c run_loop scheduler on the thread on which it starts the operation state.
 */
TEST(get_start_scheduler, sync_wait) {
    std::thread::id tid;

    auto sndr = stdexec::read_env(stdexec::get_start_scheduler) | stdexec::let_value([&](auto schd) {
                    static_assert(std::same_as<decltype(schd), stdexec::run_loop::scheduler>);
                    return stdexec::schedule(schd) | THEN_STORE_THREAD_ID(&tid);
                });

    stdexec::sync_wait(std::move(sndr)); // NOLINT(performance-move-const-arg)

    ASSERT_EQ(tid, std::this_thread::get_id());
}

/**
 * @test Check that the start scheduler that @c stdexec::let_value sets in the receiver environment
 *       of the sender returned by the closure is the completion scheduler of the predecessor.
 *
 * Indeed, @c stdexec::let_value starts the successor from the completion of the predecessor.
 *
 * See also:
 * - https://github.com/NVIDIA/stdexec/blob/5f94dbac91de3c4869fe695b7fe4d0ed66c0612d/include/stdexec/__detail/__let.hpp#L180
 * - https://github.com/NVIDIA/stdexec/blob/5f94dbac91de3c4869fe695b7fe4d0ed66c0612d/include/stdexec/__detail/__schedulers.hpp#L639-L655
 */
TEST(get_start_scheduler, let_value) {
    std::thread::id pool_tid, tid;

    experimental::execution::static_thread_pool pool{1};

    auto sndr = stdexec::schedule(pool.get_scheduler()) | THEN_STORE_THREAD_ID(&pool_tid) | stdexec::let_value([&]() {
                    return stdexec::read_env(stdexec::get_start_scheduler) | stdexec::let_value([&](auto schd) {
                               static_assert(std::same_as<decltype(schd), decltype(pool.get_scheduler())>);
                               return stdexec::schedule(schd) | THEN_STORE_THREAD_ID(&tid);
                           });
                });

    stdexec::sync_wait(std::move(sndr)); // NOLINT(performance-move-const-arg)

    ASSERT_EQ(tid, pool_tid);
    ASSERT_NE(tid, std::this_thread::get_id());
}

/**
 * @test Show that @c stdexec::continues_on onto an inline scheduler behaves as expected at run time. However, the compile-time
 *       scheduler queries are not correct: they indicate a hop onto the start scheduler set in the outer receiver environment
 *       rather than continuing on the completion scheduler of the predecessor.
 *
 * Indeed, although @c stdexec::continues_on starts an operation state from the completion of the predecessor, it does not set the start scheduler
 * accordingly through a secondary environment.
 *
 * See also:
 * - https://github.com/NVIDIA/stdexec/blob/5f94dbac91de3c4869fe695b7fe4d0ed66c0612d/include/stdexec/__detail/__continues_on.hpp#L193
 * - https://github.com/NVIDIA/stdexec/blob/5f94dbac91de3c4869fe695b7fe4d0ed66c0612d/include/stdexec/__detail/__continues_on.hpp#L127
 */
TEST(get_start_scheduler, continues_on) {
    std::thread::id pool_tid, tid;

    experimental::execution::static_thread_pool pool{1};

    auto sndr = stdexec::schedule(pool.get_scheduler()) | THEN_STORE_THREAD_ID(&pool_tid)
              | stdexec::continues_on(stdexec::inline_scheduler{}) | THEN_STORE_THREAD_ID(&tid);

    static_assert(
        std::same_as<
            stdexec::__completion_scheduler_of_t<
                stdexec::set_value_t,
                decltype(sndr),
                stdexec::prop<stdexec::get_start_scheduler_t, stdexec::run_loop::scheduler>
            >,
            stdexec::run_loop::scheduler // does not match run-time behavior; should be decltype(pool.get_scheduler())
        >);

    stdexec::sync_wait(std::move(sndr)); // NOLINT(performance-move-const-arg)

    ASSERT_EQ(tid, pool_tid); // run-time behavior is as expected
    ASSERT_NE(tid, std::this_thread::get_id());
}

Joint analysis with Claude (Anthropic).

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with include/stdexec/__detail/__continues_on.hpp at the referenced hop-opstate connection and compare it with the secondary-environment handling in let.hpp and starts_on. Run the get_start_scheduler continues_on reproducer, then verify that compile-time scheduler queries report the predecessor's completion scheduler while the runtime assertions still pass.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp
Domain
backend-api-design
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
45/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.