You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Move the task size facilities to thread_support - #8029
This is the first step of the updated plan in #8020, as suggested in #8020 (comment).
Proposed Changes
Move hpx/functional/traits/get_task_size.hpp unchanged to hpx/thread_support/task_size.hpp: task_size, task_size_duration, bulk_task_size, get_task_size, get_task_size_for and the builtin operation traits.
The function wrappers in functional (bind, bind_front, bind_back, deferred_call, one_shot, hpx::function, hpx::move_only_function) include hpx/modules/thread_support.hpp instead, and functional depends on thread_support directly.
Add the move to the V2.1.0 breaking changes. The names are unchanged; only code that included the old header directly has to include hpx/modules/thread_support.hpp instead.
Any background context you want to provide?
thread_support depends on neither functional nor coroutines, so the stack size facilities can be built there next, and thread_enums.hpp can move there later. No behavior change.
Tested with sized_function, task_size_annotations, task_size_hint, task_sizes_reduce_merge, task_size_chunk_size_configuration, executor_parameters, function_test, bind_test, launch_policy, fork_join_executor and action_task_size:
stellargroup/build_env:17 with clang-20, warnings as errors and thread debug info, as in the inspect job, including the header tests of functional and thread_support; clang-tidy-20 and the inspect tool report nothing
stellargroup/build_env:22 with clang-22, C++23 and HPX_WITH_CXX_MODULES=ON, as in the modules job
No actionable comments were generated in the recent review.
Summary
Summary by CodeRabbit
Documentation
Task-size facilities are now associated with the thread-support module, with their names unchanged.
If you include the task-size header directly, switch to the thread-support module header. The functional umbrella headers continue to provide these facilities.
Refactor
Functional utilities continue to support task-size behavior through the updated module organization; no public declarations changed.
The task-size facilities move from the functional module to thread_support without name changes. Functional headers include the thread-support module header. Release notes document the relocation and continued availability through functional umbrella headers.
The thread-support module registers task_size.hpp. The functional module adds a thread-support dependency, and its task-size consumers switch to hpx/modules/thread_support.hpp. The release notes describe the relocation and include guidance.
Annotate tasks with their stack sizes #8020: The issue proposes stack-size facilities in hpx::threads and identifies task-size facilities as a model. This PR moves task-size facilities into thread_support.
TheHPXProject/hpx#7848: This PR relocates task-size facilities introduced by PR #7848 and updates functional wrappers to include them through thread_support.
TheHPXProject/hpx#7926: Both PRs change or use the task-size API in functional wrappers.
Direct consumers must update their include as documented. No merge-blocking issue is established.
Pre-merge checks: 5
Check name
Status
Explanation
Title check
The title clearly summarizes the main change: moving task-size facilities to thread_support.
Description check
The description explains the task-size move, related include and dependency changes, and the intended compatibility impact.
Docstring Coverage
No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat…
Linked Issues check
Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check
Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage
Explanation
No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 0 functions across 9 files. (3 skipped: 3 unsupported.)
Finishing Touches
Generate unit tests (beta)
Create a new PR
Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 7 remain after this review.
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer TIP This summary will be updated as you push new changes.
The reason will be displayed to describe this comment to others. Learn more.
Thanks, this is what I had in mind. The header moves unchanged, thread_support depends on neither functional nor coroutines, so the new dependency of functional on thread_support doesn't create a cycle, and the breaking change is documented in the release notes.
The failure in the C++20 Modules build is a clang crash (exit 139) that shows up on other PRs as well, it is unrelated.
Move hpx/functional/traits/get_task_size.hpp unchanged to
hpx/thread_support/task_size.hpp: task_size, task_size_duration,
bulk_task_size, get_task_size, get_task_size_for and the builtin
operation traits. The function wrappers in functional now include
hpx/modules/thread_support.hpp, and functional depends on
thread_support directly. thread_support depends on neither functional
nor coroutines, so the stack size facilities can be added there as
well.
The names are unchanged; code that included the old header directly
has to include hpx/modules/thread_support.hpp instead. Document this in
the V2.1.0 breaking changes.
Part of TheHPXProject#8020
Signed-off-by: Nithurshen <nithurshen.dev@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Part of #8020
This is the first step of the updated plan in #8020, as suggested in #8020 (comment).
Proposed Changes
hpx/functional/traits/get_task_size.hppunchanged tohpx/thread_support/task_size.hpp:task_size,task_size_duration,bulk_task_size,get_task_size,get_task_size_forand the builtin operation traits.functional(bind,bind_front,bind_back,deferred_call,one_shot,hpx::function,hpx::move_only_function) includehpx/modules/thread_support.hppinstead, andfunctionaldepends onthread_supportdirectly.hpx/modules/thread_support.hppinstead.Any background context you want to provide?
thread_supportdepends on neitherfunctionalnorcoroutines, so the stack size facilities can be built there next, andthread_enums.hppcan move there later. No behavior change.Tested with
sized_function,task_size_annotations,task_size_hint,task_sizes_reduce_merge,task_size_chunk_size_configuration,executor_parameters,function_test,bind_test,launch_policy,fork_join_executorandaction_task_size:stellargroup/build_env:17with clang-20, warnings as errors and thread debug info, as in the inspect job, including the header tests offunctionalandthread_support; clang-tidy-20 and the inspect tool report nothingstellargroup/build_env:22with clang-22, C++23 andHPX_WITH_CXX_MODULES=ON, as in the modules job