Skip to content

fix: reclaim before accepting wrapped Vulkan iGPU memory estimates - #2124

Merged
leejet merged 2 commits into
leejet:masterfrom
losewayy:igpu-capacity-accounting
Oct 13, 2026
Merged

leejet merged 2 commits into
leejet:masterfrom
losewayy:igpu-capacity-accounting

Conversation

@losewayy

@losewayy losewayy commented Oct 10, 2026 •

Copy link
Copy Markdown
Contributor

Summary

For Vulkan integrated GPUs reporting free > total, retain the tracked-residency fallback but require reclamation before accepting it: invalid report → attempt workspace and eligible weight reclamation → accept the estimate only if the capacity limits pass.

Both the public capacity precheck and intermediate checks in ensure_compute_backend_capacity() reject early success based on this estimate. The public check also prevents callers from skipping reclamation and conservatively selects segmentation when a usable cut plan exists. Reclamation attempts are skipped when the eviction candidate set has not grown, avoiding repeated driver queries for ineligible candidates.

Addresses the invalid-report path discussed in #2022. Discrete Vulkan GPUs retain the existing rejection of free > total.

Evidence

  • Windows MSVC Release build with SD_VULKAN=ON completed; formatting checks on modified lines and git diff --check passed.
  • All 25 controlled ModelManager checks passed, covering reclamation, capacity limits, protection of pinned/protected/prefetched weights, and ordinary/discrete/non-Vulkan reports. The published version fails four reclamation checks.
  • End-to-end comparison on AMD Radeon 610M: SD 1.5, f16, mmap, disk-resident diffusion/VAE weights, 1 GiB managed budget, prefetch disabled, 256×256, two steps, CFG 1, seed 42. Normal and injected free = total + 1024 runs both exited successfully, completed sampling and VAE decoding, and saved byte-identical PNGs. The injected run recorded 45 device-weight release events. The temporary injection hook was removed and production artifacts rebuilt.

Merge Danger

The change is reversible and limited to ModelManager capacity and reclamation decisions. Existing protection filters and device/budget limits remain in place; ordinary sufficient-memory reports retain their fast path.

The fallback remains a best-effort estimate after eligible reclamation has been attempted. Artificial injection validates this execution path but does not reproduce the original Intel driver behavior or demonstrate recovery from genuine memory exhaustion. Honest low-free reports, including the Linux GTT case discussed in #2022, are outside this change.

Summary by CodeRabbit

  • Bug Fixes
    • Improved Vulkan memory availability reporting when reported free memory exceeds total memory. Integrated GPUs now account for tracked usage and indicate when memory reclamation is needed; non-integrated GPUs report no available capacity for these inconsistent readings.
    • Improved capacity checks to avoid approving a fit when memory reclamation is required, and prevented eviction attempts from continuing when they no longer change the eviction state.

@coderabbitai

coderabbitai Bot commented Oct 10, 2026 •

Copy link
Copy Markdown

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: 289a2dfb-7f70-49f5-9e46-0734e610d62d

📥 Commits

Reviewing files that changed from the base of the PR and between cdc6677 and a02df73.


📒 Files selected for processing (2)
  • src/model_manager.cpp
  • src/model_manager.h

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



📝 Walkthrough
📝 Walkthrough

Walkthrough

The Vulkan capacity check handles reports where free memory exceeds total memory. Integrated GPUs use total memory as the free-memory estimate and mark capacity as requiring reclamation. Non-integrated GPUs report zero available bytes for this condition. Capacity checks and eviction attempts also include reclamation-related conditions.

Changes

Vulkan capacity and reclamation

Layer / File(s) Summary
Device-specific capacity check
src/model_manager.h, src/model_manager.cpp
CapacityCheck adds a requires_reclamation flag. For anomalous Vulkan memory reports, the manager applies different handling for integrated and non-integrated GPUs. Capacity-fit predicates reject results marked as requiring reclamation.
Eviction and final demotion checks
src/model_manager.cpp
Eviction release attempts stop when the eviction-state count does not grow. The final check after demotion still tests fits() without checking requires_reclamation.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix · Severity of issue fixed: Medium

Suggested reviewers: leejet


Merge Risk: ⚪ Minimal · up to a02df

The wrapped-report fallback can proceed through reclamation and admit a fitting estimate. No identified issue remains that requires a fix before merge.

Architecture Summary

Architecture risk: 🟡 Medium · up to a02df

The change affects 1 system.

Changed systems: src

Architecture concerns
No architecture-level concerns identified.

Review details

Systems and components

  • observed — src (service) was modified; 2 changed files map to changed impact.

Before / after behavior

  • observed — Modified behavior in src/model_manager.cpp: For Vulkan reports where free memory exceeds total memory, non-integrated GPUs now report zero available bytes; integrated GPUs use total memory as the free-memory estimate and mark capacity as requiring reclamation.
  • observed — Modified behavior in src/model_manager.cpp: fits_compute_backend_capacity now resolves states separately and returns false if capacity requires reclamation, even when the capacity otherwise fits.
  • observed — Modified behavior in src/model_manager.cpp: The capacity predicate used during reclamation now requires both that capacity fits and that reclamation is not required.
  • observed — Modified behavior in src/model_manager.cpp: Eviction release attempts now return false when the eviction-state count has not grown since the previous attempt; otherwise they record the new count before releasing blocks.

Reliability and maintainability

  • inferred — Risk-relevant change factors for src: blast_radius_1; direct_dependents_1

Pre-merge checks · Passed 3 · Failed 2

Failed Failed checks (2 warnings)

Check name Status Explanation Resolution
Linked Issues check Warning The change addresses the #2022 Vulkan UMA underflow path. check_capacity() marks an integrated-GPU free_bytes > total_bytes report with requires_reclamation. The public precheck and the initial … Make every success path in ensure_compute_backend_capacity() require both capacity.fits() and !capacity.requires_reclamation, including the post-demotion check. Add or update an automated test that exercises an impossible integrated-G…
Docstring Coverage Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 2 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks (3 passed)
Check name Status Explanation
Out of Scope Changes check Passed The changes are limited to ModelManager capacity state and reclamation flow in src/model_manager.cpp and src/model_manager.h. The changes support [#2022] by handling the Vulkan UMA underflow rep…
Description check Passed The description clearly explains the change, rationale, validation evidence, scope, and limitations. It does not use every template heading, but it provides the required information in equivalent sect…
Title check Passed The title is concise, specific, and accurately describes the main change: reclaiming memory before accepting wrapped Vulkan integrated-GPU estimates.

Full details: Linked Issues check

Explanation

The change addresses the #2022 Vulkan UMA underflow path. check_capacity() marks an integrated-GPU free_bytes > total_bytes report with requires_reclamation. The public precheck and the initial ensure_compute_backend_capacity() check reject that estimate. However, the post-demotion success path at src/model_manager.cpp accepts check_capacity(...).fits() without checking requires_reclamation. That path can accept the tracked estimate without the required reclamation. The linked issue objective is therefore not fully implemented. The reported tests do not establish coverage of this post-demotion path.

Resolution

Make every success path in ensure_compute_backend_capacity() require both capacity.fits() and !capacity.requires_reclamation, including the post-demotion check. Add or update an automated test that exercises an impossible integrated-GPU report through this path and verifies that reclamation occurs before success.


  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR

  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

@leejet leejet left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

free_bytes > total_bytes can also occur under genuine memory pressure when heap usage exceeds its budget. Replacing it with total_bytes can make fits() return true and bypass workspace/weight eviction in ensure_compute_backend_capacity(). If the subsequent allocation fails, the segment aborts without retrying after reclamation. This can regress workloads that previously succeeded after evicting cached allocations.

Could we retain the fallback but reclaim eligible allocations and retry on allocation failure, or attempt reclamation before accepting this optimistic estimate?

@losewayy losewayy changed the title fix: fall back to tracked residency when Vulkan iGPU free report wraps fix: reclaim before accepting wrapped Vulkan iGPU memory estimates Oct 12, 2026
@losewayy

Copy link
Copy Markdown
Contributor Author

Changed it to reclaim eligible workspaces and cached weights before accepting the estimate, including through the public capacity precheck. The Vulkan build and all 25 local checks pass. SD 1.5 completes on the Radeon 610M with both normal and injected free > total reports, producing identical PNGs. I haven't reproduced the Intel driver issue naturally.

@losewayy
losewayy requested a review from leejet October 13, 2026 09:00
@leejet
leejet merged commit 60423ab into leejet:master Oct 13, 2026
11 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.

2 participants