-
Notifications
You must be signed in to change notification settings - Fork 667
fix(v1,serve): env-worker memory — step slimming, registry eviction, intercept discard, worker trim #1608
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(v1,serve): env-worker memory — step slimming, registry eviction, intercept discard, worker trim #1608
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -875,10 +875,21 @@ async def submit_model_request( | |
| is_truncated = response.message.is_truncated or ( | ||
| tokens is not None and bool(tokens.get("is_truncated")) | ||
| ) | ||
| # Identity/usage only: the full Response dump would re-store the | ||
| # message content (= ``completion``) and ``message.tokens`` | ||
| # (= ``tokens``, incl. per-token attribution), doubling every | ||
| # trajectory step in worker memory, on the wire, and in the | ||
| # orchestrator's group buffers. v1 step readers that want the | ||
| # heavy fields isinstance-check the live ``Response``, which a | ||
| # serialized dict never passes; usage is recorded on state at | ||
| # this call site via ``record_response_usage``. | ||
| response_meta = serializable(response) | ||
| if isinstance(response_meta, dict): | ||
| response_meta.pop("message", None) | ||
| step = { | ||
| "prompt": serializable(prompt), | ||
| "completion": serializable(completion), | ||
| "response": serializable(response), | ||
| "response": response_meta, | ||
| "tokens": serializable(tokens), | ||
| "reward": None, | ||
| "advantage": None, | ||
|
|
@@ -978,6 +989,12 @@ async def cleanup_rollout(self, task: Task, state: State) -> None: | |
| await self.close_mcp_tools(state) | ||
| self.release_scoped_tools("rollout", state) | ||
| await self.release_model_client(state) | ||
| # The live-trajectory registry (register_trajectory) is only read by | ||
| # resolve_trajectory for handle-borrowing sub-runtime states, whose | ||
| # lifetime is within the owning rollout — without this pop the | ||
| # long-lived Runtime retains every completed rollout's full | ||
| # trajectory and the env worker leaks ~50-400MB per rollout. | ||
| self.trajectories.pop(str(state["trajectory_id"]), None) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
For grouped rollouts this cleanup runs before the group phase: Useful? React with 👍 / 👎. |
||
| self.release_tool_handles(state) | ||
|
|
||
| async def cleanup_group(self, tasks: list[Task], states: list[State]) -> None: | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When renderer rollouts hit training-budget truncation,
renderer_client._step_token_idsintentionally falls back from emptystep["tokens"].completion_idstostep["response"]["message"]["tokens"](seeverifiers/clients/renderer_client.pyand the fallback tests intests/test_renderer_client.py). Removingmessagehere makes that fallback returnNone, so subsequent turns are fully re-rendered instead of bridged and can lose the cache/multimodal placeholder continuity the renderer path relies on for long multimodal rollouts. If the goal is to drop duplicated content, keep a minimal raw-token/is-truncated sidecar rather than deleting the whole message.Useful? React with 👍 / 👎.