Skip to content

Preserve launch properties for future continuations - #7818

Open
the-ivii wants to merge 2 commits into
TheHPXProject:masterfrom
the-ivii:fix/continuation-launch-policy-7799
Open

the-ivii wants to merge 2 commits into
TheHPXProject:masterfrom
the-ivii:fix/continuation-launch-policy-7799

Conversation

@the-ivii

@the-ivii the-ivii commented Oct 21, 2026 •

Copy link
Copy Markdown
Contributor

Fixes #7799

Proposed Changes

  • Preserve a continuation launch policy's priority, scheduling hint, and stack size when creating its HPX thread.
  • Publish run-as-child continuation threads while suspended, then make them pending after the shared state safely owns the thread ID.
  • Let continuation waits and result retrieval directly execute an eligible child thread, with regression coverage for policy propagation and inline execution.

Any background context you want to provide?

Validated with AppleClang 21 in Release mode:

Checklist

Not all points below apply to all pull requests.

  • I have added a new feature and have added tests to go along with it.
  • I have fixed a bug and have added a regression test.
  • I have added a test using random numbers; I have made sure it uses a seed, and that random numbers generated are valid inputs for the tests.

@the-ivii
the-ivii requested a review from hkaiser as a code owner October 21, 2026 08:53
@coderabbitai

coderabbitai Bot commented Oct 21, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 77f3ee13-2840-4150-87ed-109398b80e64
📥 Commits

Reviewing files that changed from the base of the PR and between 218aa2f and d154912.

📒 Files selected for processing: 4
  • libs/core/execution/include/hpx/execution/detail/future_exec.hpp
  • libs/core/futures/include/hpx/futures/packaged_continuation.hpp
  • libs/core/futures/tests/regressions/CMakeLists.txt
  • libs/core/futures/tests/regressions/continuation_launch_policy_7799.cpp

Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 7 remain after this review.

📝 Summary

Summary by CodeRabbit

  • New Features

    • Continuations now honor scheduling policies, including worker selection, priority, stack size, and thread-pool preferences.
    • Child continuations can execute inline on the parent worker when direct execution is supported.
    • Deferred continuations now execute correctly when results are retrieved or futures are waited on.
  • Bug Fixes

    • Improved continuation scheduling and execution behavior across supported launch policies.
  • Tests

    • Added regression coverage for continuation launch policies and child execution timing.

Continuation launch paths now pass scheduling policies to spawners and support child execution. Continuation wait and result operations attempt execution before delegating to the base future state. A regression test checks launch properties and run-as-child execution.

Changes: Continuation Launch Policy

Layer / File(s) Summary
Policy-aware continuation spawning
libs/core/execution/include/hpx/execution/detail/future_exec.hpp, libs/core/futures/include/hpx/futures/packaged_continuation.hpp
Spawners receive the launch policy. The policy spawner uses its scheduling settings and starts child-mode threads after the continuation records the thread ID. The asynchronous attach path forwards the policy.
Inline execution and regression coverage
libs/core/futures/include/hpx/futures/packaged_continuation.hpp, libs/core/futures/tests/regressions/continuation_launch_policy_7799.cpp, libs/core/futures/tests/regressions/CMakeLists.txt
Continuation wait and result operations attempt execution before delegating to the base state. The regression test checks worker selection, priority, stack size, and run-as-child execution; CMake registers it with two threads per locality.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix

Sequence Diagram(s)

sequenceDiagram
  participant Caller
  participant continuation
  participant post_policy_spawner
  continuation->>post_policy_spawner: operator() with policy
  post_policy_spawner-->>continuation: child-mode result and thread ID
  continuation->>continuation: Store thread ID under mutex
  continuation->>post_policy_spawner: start(thread ID, policy)
  Caller->>continuation: wait() or get_result_void()
  continuation->>continuation: Attempt execute_thread()
Loading
Merge Risk: ⚪ Minimal · up to d1549

The reviewed continuation paths preserve the intended inline-execution behavior, with no identified issue that should delay merging.

Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 11.76% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 17 functions across 3 files. (1 skipped: … Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 passed
Check name Status Explanation
Title check Passed The title directly and concisely describes the primary change: preserving launch properties for future continuations.
Linked Issues check Passed Issue #7799 has coding requirements for policy propagation, synchronized run-as-child publication, inline continuation execution, and executor-path preservation. The diff passes policy.priority(), `…
Out of Scope Changes check Passed The changes stay within issue #7799. The implementation updates continuation spawning and wait paths, preserves executor-based continuation handling, and adds focused regression-test registration and …
Description check Passed The pull request description directly matches the changes. It covers launch-policy propagation, suspended child-thread publication, inline continuation execution, and regression testing.
Full details: Linked Issues check

Explanation

Issue #7799 has coding requirements for policy propagation, synchronized run-as-child publication, inline continuation execution, and executor-path preservation. The diff passes policy.priority(), policy.hint(), and policy.stacksize() to thread_init_data. It registers run-as-child threads suspended, publishes runs_child_ under mtx_, and then starts the thread as pending. Continuation wait, timed wait, result retrieval, and deferred execution call execute_thread(). Executor spawners retain the executor path. The new regression test checks worker selection, bound priority, medium stack size, and inline execution.

Full details: Out of Scope Changes check

Explanation

The changes stay within issue #7799. The implementation updates continuation spawning and wait paths, preserves executor-based continuation handling, and adds focused regression-test registration and coverage. No unrelated product behavior or unrelated files appear in the reviewed changes.

Full details: Docstring Coverage

Explanation

Docstring coverage is 11.76% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 17 functions across 3 files. (1 skipped: 1 unsupported.)

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

✨ 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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@codacy-production

Copy link
Copy Markdown

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

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.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Continuations ignore the launch policy's priority, hint and stacksize, and never run as child

2 participants