Implement P2300 get_scheduler bridge for executors#7238
Conversation
Up to standards ✅🟢 Issues
|
|
Can one of the admins verify this patch? |
75fba86 to
d384fd9
Compare
|
@shivansh023023 This is really a very interesting PR connecting our executor infrastructure to senders&receivers. Thanks for this. |
hkaiser
left a comment
There was a problem hiding this comment.
Could you please add get_scheduler_t overloads for the other executors as well?
|
Thanks for the review, @hkaiser ! I've gone ahead and addressed your suggestions: Modernization: Refactored the executor_scheduler constructor to use a C++20 requires clause, replacing the previous SFINAE/enable_if_t pattern. Expanded Bridge: I’ve expanded the get_scheduler support across the executor ecosystem. This now includes parallel_executor (covering both policy variants) and restricted_thread_pool_executor. Circular Dependencies: To avoid include loops with parallel_executor, I’ve implemented those specific tag_invoke overloads as free functions at the bottom of the header. Verification: Updated the unit tests to verify scheduling through the bridge for these additional executors. Locally, executor_scheduler_test is passing cleanly. |
|
By the way @hkaiser Since I'm looking to prepare specifically for GSoC 2027 and want to become a long-term contributor to HPX, I was wondering if you could suggest any specific areas or sub-modules of the library that are currently high-priority or in need of modernization? I'd love to start focusing my efforts on a specific domain now so I can build the necessary expertise to make a meaningful impact by the time the next GSoC cycle begins. Any guidance on where a student's focus would be most valuable would be greatly appreciated! |
There was a problem hiding this comment.
The new executor_scheduler should be adapted to the bulk family of facilities. All of our executors support bulk operations, so should the scheduler that derives from them (I just now realized, is that what you proposed in #7240)?
|
@shivansh023023 Could you please have a look at the compilation problems reported? |
I think that the sender&receiver domain will stay important for a while, especially since the standard libraries will soon start shipping with their own implementation (in which case we have to adapt our code base, etc.). |
|
@hkaiser Regarding the executor_scheduler: I’m currently addressing the compilation failures reported by the CI for #7238. It appears to be a header self-containment issue on certain platforms. I’ll push the fixes shortly to get this back to green. I’ll also take the discussion regarding specific GSoC 2027 focus areas to the Discord channel once these PRs are stabilized, to avoid cluttering the review thread. |
29accc5 to
3b9fa73
Compare
Remove the stray target_compile_definitions(executor_scheduler_test ...) that was placed outside both the foreach loop and the if(NOT Clang) guard. On Clang (and on any configure path that skips the foreach), the target does not exist, causing a CMake fatal error that broke every CI job. The definition was redundant: add_hpx_executable with INTERNAL_FLAGS already sets HPX_APPLICATION_NAME for all test targets. Also restore HPX-standard 2-space CMake indentation (was 4-space after a bad merge conflict resolution). Refs: TheHPXProject#7238
|
@hkaiser |
Remove the stray target_compile_definitions(executor_scheduler_test ...) that was placed outside both the foreach loop and the if(NOT Clang) guard. On Clang (and on any configure path that skips the foreach), the target does not exist, causing a CMake fatal error that broke every CI job. The definition was redundant: add_hpx_executable with INTERNAL_FLAGS already sets HPX_APPLICATION_NAME for all test targets. Also restore HPX-standard 2-space CMake indentation (was 4-space after a bad merge conflict resolution). Refs: TheHPXProject#7238
842875f to
df7fcac
Compare
|
@hkaiser The headers should now be correctly visible through C++ module BMIs! |
|
@shivansh023023 I'd like for #7274 to be resolved first before starting to merge even more S&R functionalities. If you have free cycles, please feel free to help with figuring out what's wrong there. We never noticed issues with the new S&R implementations as many tests got accidentally disabled. |
|
I believe you are referring to #7074 (the Godbolt integration PR). I completely agree,I'll prioritize resolving that one immediately before we move forward with the additional S&R bridge work. I'm currently rebasing #7074 to remove the redundant local.hpp header (which we already addressed in #7070) and cleaning up the configuration logic to ensure the CI is green. I'll ping you as soon as it's ready for a final look! |
|
@hkaiser All green on this pr, implemented your feedback. Let me know when you can take another look! |
Sorry, I was referring to #7257 (which has been merged now). |
Up to standards ✅🟢 Issues
|
92e5e24 to
9a1b10b
Compare
|
I have fully migrated executor_scheduler.hpp away from tag_invoke. start, connect, schedule, and get_completion_signatures are now all implemented as native member functions with the correct qualifiers per the latest P2300 specs. |
|
@shivansh023023 Could you plae pay attention to the CIs yourself? Those currently report various compilation (and formatting) problems. |
|
@shivansh023023 I think this PR should be the next in line for merging. It implements basic functionalities needed by some of the other of your PRs. Would you be able to clean up the remaining issues here so we can move ahead? |
e2acc92 to
00071b6
Compare
hkaiser
left a comment
There was a problem hiding this comment.
Sorry for spamming you with comments...
|
|
||
| /////////////////////////////////////////////////////////////////////////// | ||
| HPX_CXX_CORE_EXPORT template <typename Executor, typename Receiver> | ||
| struct executor_operation_state |
There was a problem hiding this comment.
Please move the executor_operation_state and the executor_sender into namespace detail. Perhaps even the executor_scheduler. Those types will never be explicitly named by the user.
| parallel_policy_executor( | ||
| parallel_policy_executor const&) noexcept = default; | ||
| parallel_policy_executor(parallel_policy_executor&&) noexcept = default; | ||
| parallel_policy_executor& operator=( | ||
| parallel_policy_executor const&) = default; | ||
| parallel_policy_executor const&) noexcept = default; | ||
| parallel_policy_executor& operator=( | ||
| parallel_policy_executor&&) = default; | ||
| parallel_policy_executor&&) noexcept = default; |
There was a problem hiding this comment.
These functions are noexcept by default. There is no need to explicitly say that.
|
|
||
| restricted_policy_executor(restricted_policy_executor const& other) | ||
| restricted_policy_executor( | ||
| restricted_policy_executor const& other) noexcept |
There was a problem hiding this comment.
Are you sure that the copy-constructor of the embedded executor is always noexcept?
|
|
||
| restricted_policy_executor& operator=( | ||
| restricted_policy_executor const& rhs) | ||
| restricted_policy_executor const& rhs) noexcept |
| constexpr executor_scheduler() = default; | ||
|
|
||
| template <typename Exec, | ||
| typename = std::enable_if_t< |
There was a problem hiding this comment.
Please use requires() instead of SFINAE, if possible.
| { | ||
| hpx::detail::try_catch_exception_ptr( | ||
| [&]() { | ||
| hpx::parallel::execution::post( |
There was a problem hiding this comment.
Is this the correct solution for the sequenced_executor? Shouldn't the operation state for that one simply call set_value() inline? You should be able to use
std::is_same_v<
hpx::execution::experimental::executor_execution_category_t<Executor>,
hpx::execution::sequenced_executoion_tag>`
to find out.
| # Essential definitions | ||
| target_compile_definitions( | ||
| executor_scheduler_test PRIVATE HPX_APPLICATION_NAME=executor_scheduler_test | ||
| ) |
There was a problem hiding this comment.
This should automatically be defined by the build system.
| "executor_scheduler must satisfy is_scheduler"); | ||
|
|
||
| // Create a sender pipeline: schedule | then | ||
| auto s = then(schedule(sched), []() { return 42; }); |
There was a problem hiding this comment.
Please make sure the lambda is invoked by the calling thread. You could return the hpx::thread::id instead of the integer and compare after execution.
| find_library(TBB_LIBRARY NAMES tbb) | ||
| if(TBB_LIBRARY) | ||
| target_link_libraries(hpx_core PUBLIC ${TBB_LIBRARY}) | ||
| endif() |
There was a problem hiding this comment.
I think we agreed to remove the TBB dependency.
| endif() | ||
|
|
||
| if(TARGET STDEXEC::stdexec) | ||
| target_link_libraries(hpx_core INTERFACE STDEXEC::stdexec) |
There was a problem hiding this comment.
I believe this can be removed now as well.
|
@hkaiser I also want to sincerely apologize for the messy state of this PR and the back-and-forth. To be completely transparent, I relied heavily on an LLM to help me navigate the complex template metaprogramming and P2300 transitions. While it helped me get a foothold, it clearly also introduced some clumsy mistakes, redundant code (like the noexcept defaults), and weird Git auto-merges that I failed to catch. I am actively using your feedback to truly understand how HPX's execution architecture works under the hood, and I am slowly shifting away from the AI to write and verify things manually. I really appreciate your patience and mentorship while I climb this learning curve! I am implementing every single point of your feedback right now (moving internals to the detail namespace, swapping SFINAE for C++20 requires, removing the old CMake links, etc.). I will ping you as soon as the code is clean and the CI is green! |
7ae6212 to
ff0976d
Compare
Add executor_scheduler adapter in hpx::execution::experimental that provides a P2300-compatible scheduler interface for existing HPX executors. The adapter consists of: - executor_scheduler: wraps any HPX executor into a P2300 scheduler with schedule() returning a sender - detail::executor_sender: sender that dispatches work via the underlying executor - detail::executor_operation_state: operation state with sequenced optimization (inline set_value for sequenced_execution_tag) Add query(get_scheduler_t) and schedule() member functions to: - sequenced_executor - parallel_policy_executor (both specializations, out-of-line) - restricted_policy_executor Uses C++20 requires clause instead of SFINAE. Conditional noexcept on restricted_policy_executor copy operations. Includes unit test verifying schedule/then pipeline with thread identity checks. Signed-off-by: Shivansh Singh <singhshivansh023@gmail.com> Signed-off-by: Shivansh Singh <your_github_email@example.com>
ff0976d to
9c25942
Compare
PR Description: Implement P2300
get_schedulerBridge for HPX ExecutorsFixes
This PR addresses the ongoing effort to align HPX Executors with the C++23 P2300 (std::execution) proposal.
Proposed Changes
executor_scheduleradapter: Implemented a new bridge headerlibs/core/executors/include/hpx/executors/executor_scheduler.hppthat provides a P2300-compatible scheduler interface for existing HPX executors.get_schedulerforsequenced_executor: Added atag_invokeoverload forhpx::execution::experimental::get_schedulerinsequenced_executor.hpp, allowing it to be used directly in sender/receiver pipelines.executor_scheduler_senderandexecutor_scheduler_operation_stateto manage task dispatching via the executor.libs/core/executors/tests/unit/executor_scheduler.cppto verify the end-to-end flow:schedule(get_scheduler(exec)) | then(...).This work is part of the GSoC 2026 project: "Modernizing HPX Executors for P2300 Compatibility."
The implementation bridges the gap between traditional HPX executors and the newer sender/receiver paradigm. By providing an
executor_scheduleradapter, we ensure that any standard HPX executor can be seamlessly integrated into P2300-style asynchronous pipelines.Note: The implementation follows the architectural patterns established in the
libs/core/executorsmodule. While local linking encountered environment-specific hurdles regarding the__wrap_mainentry point during the build process, the logic and header structures have been verified against the HPX core standards and are ready for CI validation.Checklist