Let tests skip the settle wait between retried VRAM reads - #9141
Conversation
settled_snapshot_device_memory spaces its retried mem_get_info reads a second apart so a transient tenant on a live card has time to clear. Under test the snapshots are stubs whose answers do not change with time, so the wait buys nothing. Tracing time.sleep through one test showed 4.00s of a 4.03s call coming from diffusion_memory.py:244, reached via _plan_memory, which has no way to forward delay_s. The tests that call the function directly already pass delay_s = 0. Adds UNSLOTH_SETTLE_DELAY_S, defaulted to 0 in the backend conftest. Production is untouched: with the variable unset the caller's delay is returned as-is, and an unparseable or negative value falls back rather than being read as do-not-wait. Scope, measured rather than assumed. Back to back on the four affected files at -n 4: 68.44s with the production spacing, 39.44s without, 639 passed both times. On the FULL backend suite at -n 4 the effect is inside run-to-run noise on this box (259s / 326s / 328s across three runs), because sleeping workers do not hold a core and the suite is bound by its longest worker rather than by total CPU. So this is a real saving for a targeted run of the diffusion and video files and for any serial run, and it is not a Backend CI number. The full-suite failure set is unchanged: same 34, plus the two known-flaky tool_output_streaming grandchild timeouts that were red in the baseline and green here.
for more information, see https://pre-commit.ci
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bb0568cd14
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| if override is None: | ||
| return delay_s | ||
| try: | ||
| return max(0.0, float(override)) |
There was a problem hiding this comment.
When UNSLOTH_SETTLE_DELAY_S is set to a value such as inf or nan, float() accepts it rather than taking the invalid-value fallback. With inf, time.sleep() raises OverflowError, which the surrounding retry loop catches by breaking before any subsequent VRAM reads; with nan, this expression resolves to zero and silently removes the production wait. Validate that the parsed value is finite and otherwise return delay_s, so a malformed override cannot change retry behavior.
Useful? React with 👍 / 👎.
1 participant
settled_snapshot_device_memory spaces its retried mem_get_info reads a second
apart so a transient tenant on a live card has time to clear. Under test the
snapshots are stubs whose answers do not change with time, so the wait buys
nothing. Tracing time.sleep through one test showed 4.00s of a 4.03s call coming
from diffusion_memory.py:244, reached via _plan_memory, which has no way to
forward delay_s. The tests that call the function directly already pass
delay_s = 0.
Adds UNSLOTH_SETTLE_DELAY_S, defaulted to 0 in the backend conftest. Production
is untouched: with the variable unset the caller's delay is returned as-is, and
an unparseable or negative value falls back rather than being read as do-not-wait.
Scope, measured rather than assumed. Back to back on the four affected files at
-n 4: 68.44s with the production spacing, 39.44s without, 639 passed both times.
On the FULL backend suite at -n 4 the effect is inside run-to-run noise on this
box (259s / 326s / 328s across three runs), because sleeping workers do not hold
a core and the suite is bound by its longest worker rather than by total CPU. So
this is a real saving for a targeted run of the diffusion and video files and
for any serial run, and it is not a Backend CI number. The full-suite failure set
is unchanged: same 34, plus the two known-flaky tool_output_streaming grandchild
timeouts that were red in the baseline and green here.