Skip to content

docs: name concurrent iterations as the cause when the residency budget starves - #39

Closed
emfdavid wants to merge 1 commit into
pool-lifecycle-phase-afrom
budget-concurrent-iterations
Closed

docs: name concurrent iterations as the cause when the residency budget starves#39
emfdavid wants to merge 1 commit into
pool-lifecycle-phase-afrom
budget-concurrent-iterations

Conversation

@emfdavid

Copy link
Copy Markdown
Owner

Summary

Refs #38 — settles its open question and closes the two items that follow from it.
Stacked on #37 (base is pool-lifecycle-phase-a); merge that first.

One InSituDataset owns one ChunkPool and every active iteration shares it —
zip(ds.train, ds.val), or two DataLoaders — but each holds its own chunk
references, so residency is the sum of their working sets, not the maximum.

The default stays sized for one iteration. The engine cannot know how many you intend
to run, so any automatic multiplier is a guess that silently costs memory in the
single-iteration case, which is almost every case. Running several is an explicit choice,
so sizing for it is the caller's.

What was actually missing is the diagnostic. Starvation advised "raise
cache_budget_bytes, or lower batch_size / block_chunks"
— correct, but not actionable
when every resident chunk is legitimately referenced and the caller has no way to see
why. It now names how many iterations share the pool and the pattern that produces that:

... every one of them pinned or in flight; no tile is in flight; and the consumer is
blocked waiting on [('t2m', 0)]. Nothing can free a slot, so this would hang. The working
set is larger than the budget it was sized for -- raise cache_budget_bytes, or lower
batch_size / block_chunks. NOTE: 2 iterations are sharing this pool (e.g. `zip(ds.train,
ds.val)`, or two DataLoaders); each needs its own working set resident at once, so the
budget must cover all 2.

Plus docs/tuning.md §"Several iterations at once multiply the budget" with the sizing
rule and why the default is not multiplied, and the caveat at the auto-sizing site and on
the docstring that advertises the shared-pool configuration — so it is not only in prose.

For reviewers

active_owners counts from mint to release, not from first pin. My first
implementation counted owners currently holding a reference, and the test caught it: the
second iteration starves before it can pin anything, so the count read 1 and the note
never fired — missing precisely the case it exists for.

No behavior change beyond the message text: the default budget, the eviction predicate and
the admission path are untouched.

Author attestation

  • I have reviewed every change in this PR, I can explain why each one is correct, and I
    have verified the claims made in this description.

Left unchecked deliberately — drafted by Claude; that box is the human author's.

Checklist

  • Tests added or updated — test_starvation_names_concurrent_iterations_as_the_cause
    drives the real two-iteration deadlock and asserts the count and the pattern
  • ruff, mypy, pytest -q green locally — 299 passed / 20 skipped, against a 298 /
    20 baseline on the same tree with the same extras (the delta is exactly the new test)
  • Docstrings and API docs for any new or changed public surface
  • User-facing behavior documented in docs/*.md
  • A bullet added under ## Unreleased in CHANGELOG.md
  • No load-bearing invariant is broken
  • Touches ChunkPool → free-threaded run passes: 70 passed, 1 skipped on 3.13t with
    PYTHON_GIL=0
  • No performance claim.

…et starves

One InSituDataset owns one ChunkPool and every active iteration shares it, but each
holds its own chunk references -- so residency is the sum of their working sets, not
the maximum. Sizing the default for ONE iteration is the right default and stays:
the engine cannot know how many iterations a caller intends to run, and guessing
high would cost memory in the single-iteration case that is almost every case.

What was missing is the diagnostic. Starvation advised "raise cache_budget_bytes, or
lower batch_size / block_chunks" -- correct, but not actionable when every resident
chunk is legitimately referenced and the caller cannot see why. It now names how
many iterations share the pool and the pattern that produces that.

Owners count from mint to release rather than from their first pin: the iteration
that starves BEFORE it can pin anything is exactly the one that needs naming, and
counting pin-holders misses it (caught by the test, which failed on the first
implementation).

docs/tuning.md gains the sizing rule and the reason the default is not multiplied.
The auto-sizing site and the docstrings that advertise the shared-pool configuration
(buffers.BatchBuffers) carry the caveat, so it is not only in the prose.

Refs #38

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WxdT3e62pYT3EVMBF1sA7C
@emfdavid

Copy link
Copy Markdown
Owner Author

Folding into #37 instead — same review, one PR.

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.

1 participant