Skip to content

bound and configure rollout prompt prefetch - #716

Merged
lfengad merged 4 commits into
nvidia-cosmos:mainfrom
chenyx09:agent/configurable-prefetch-queue-maxsize
Jul 31, 2026
Merged

bound and configure rollout prompt prefetch#716
lfengad merged 4 commits into
nvidia-cosmos:mainfrom
chenyx09:agent/configurable-prefetch-queue-maxsize

Conversation

@chenyx09

@chenyx09 chenyx09 commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Add rollout.prefetch_queue_maxsize with default 2 and ge=1 validation.
  • Construct the bounded prompt queue after rollout backend post_init_hook overrides are visible.
  • Reuse the configured capacity for multi-rank prep-overlap target depth while preserving legacy depth 1 when overlap is disabled.
  • In the single-process prefetch loop, wait for queue capacity before leasing another controller prompt or starting its setup.

Why

Queue put() backpressure happens too late: the prefetch producer has already leased a controller prompt and submitted its setup before it blocks on a full queue. This can leave one additional dispatched batch hidden outside the configured queue capacity.

Because queued prompts consume the controller's samples_on_the_fly budget, deeper queues can also engage the soft throttle earlier. Values above 2 are therefore intended only for absorbing bursty fetch/setup latency.

Validation

  • uv run python tests/test_rollout_prefetch_loop_integration.py — 13 tests passed
  • uvx ruff@0.12.7 check --fix cosmos_rl/policy/config/__init__.py cosmos_rl/rollout/worker/rollout_control.py tests/test_rollout_prefetch_loop_integration.py
  • uvx ruff@0.12.7 format cosmos_rl/policy/config/__init__.py cosmos_rl/rollout/worker/rollout_control.py tests/test_rollout_prefetch_loop_integration.py
  • python -m py_compile cosmos_rl/policy/config/__init__.py cosmos_rl/rollout/worker/rollout_control.py tests/test_rollout_prefetch_loop_integration.py
  • git diff --check

@shuangwu shuangwu left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed the change end to end. The refactor itself is clean and the ge=1 validation is right, but there are three things worth addressing before this lands.

On the premise: is > 2 actually useful?

maxsize=2 already means "the prefetcher is exactly one batch ahead", which is enough to fully hide fetch/setup latency whenever generation dominates — and it normally does. Going deeper doesn't buy more overlap; it just parks more prompts in front of a consumer that can only take one batch at a time. The one case where a deeper queue genuinely helps is bursty fetch/setup latency (high variance rather than high mean), where a second spare batch absorbs an outlier. That's a narrow case, and I think the field description should say so — otherwise people will raise this expecting throughput and get the costs in comment 3 instead.

Happy to be wrong if you have a workload where a deeper queue measurably helps — a before/after number would settle it and would be worth putting in the field description.

The three inline comments below are the actionable part.

Comment thread cosmos_rl/rollout/worker/rollout_control.py Outdated
Comment thread tests/test_rollout_prefetch_loop_integration.py Outdated
Comment thread cosmos_rl/policy/config/__init__.py Outdated
@chenyx09 chenyx09 changed the title make rollout prefetch queue size configurable bound and configure rollout prompt prefetch Jul 29, 2026
@chenyx09
chenyx09 marked this pull request as ready for review July 30, 2026 16:06
@lfengad
lfengad merged commit 89f2e5b into nvidia-cosmos:main Jul 31, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants