Comfy Aimdo 0.4.10 + Dynamic --reserve-vram + --vram-headroom - #14480
Conversation
Implement --vram-headroom for dynamic vram as a hybrid debug/diagnostic option that can be used for people who still report shared VRAM spills. They can trial and error the setting to maintain a bit more headroom to avoid shared VRAM spills.
📝 WalkthroughWalkthroughA new 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@comfy/cli_args.py`:
- Line 148: The --vram-headroom argument in the parser.add_argument call accepts
unbounded float values without validation, allowing negative values that can
cause inverted behavior in byte conversions downstream in main.py. Add a
non-negative validator to the argument definition using argparse's type
parameter or choices mechanism to ensure only non-negative float values are
accepted at parse time, preventing invalid negative headroom values from
reaching the DynamicVRAM logic.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: dc81ab66-eca6-4e74-9b4a-a560e345b849
📥 Commits
Reviewing files that changed from the base of the PR and between 7d4194d and bdf1ffc744905069ead89736837e2fa30ae2d557.
📒 Files selected for processing (3)
comfy/cli_args.pymain.pyrequirements.txt
| @@ -145,6 +145,7 @@ def from_string(cls, value: str): | |||
| vram_group.add_argument("--cpu", action="store_true", help="To use the CPU for everything (slow).") | |||
|
|
|||
| parser.add_argument("--reserve-vram", type=float, default=None, help="Set the amount of vram in GB you want to reserve for use by your OS/other software. By default some amount is reserved depending on your OS.") | |||
| parser.add_argument("--vram-headroom", type=float, default=0, help="Set the amount of vram in GB for DynamicVRAM to maintain as extra headroom above default. ComfyUI will try and keep this much VRAM completely free and unused, even counting VRAM from other apps.") | |||
There was a problem hiding this comment.
Validate VRAM headroom inputs as non-negative at parse time.
Line 148 introduces an unbounded float; negative values flow into byte conversions in main.py (Lines 58 and 234), which can invert headroom behavior under DynamicVRAM. Add a non-negative validator for VRAM headroom inputs.
Suggested fix
+def non_negative_gb(value: str) -> float:
+ gb = float(value)
+ if gb < 0:
+ raise argparse.ArgumentTypeError("Value must be >= 0 GB.")
+ return gb
+
-parser.add_argument("--reserve-vram", type=float, default=None, help="Set the amount of vram in GB you want to reserve for use by your OS/other software. By default some amount is reserved depending on your OS.")
-parser.add_argument("--vram-headroom", type=float, default=0, help="Set the amount of vram in GB for DynamicVRAM to maintain as extra headroom above default. ComfyUI will try and keep this much VRAM completely free and unused, even counting VRAM from other apps.")
+parser.add_argument("--reserve-vram", type=non_negative_gb, default=None, help="Set the amount of vram in GB you want to reserve for use by your OS/other software. By default some amount is reserved depending on your OS.")
+parser.add_argument("--vram-headroom", type=non_negative_gb, default=0, help="Set the amount of vram in GB for DynamicVRAM to maintain as extra headroom above default. ComfyUI will try and keep this much VRAM completely free and unused, even counting VRAM from other apps.")As per coding guidelines, comfy/** changes should prioritize memory-management safety and backward-compatible behavior.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@comfy/cli_args.py` at line 148, The --vram-headroom argument in the
parser.add_argument call accepts unbounded float values without validation,
allowing negative values that can cause inverted behavior in byte conversions
downstream in main.py. Add a non-negative validator to the argument definition
using argparse's type parameter or choices mechanism to ensure only non-negative
float values are accepted at parse time, preventing invalid negative headroom
values from reaching the DynamicVRAM logic.
Source: Coding guidelines
Implement --reserve-vram as extra headroom on the simple method which is semantically as close as possible to the stated functionality and formet behaviour of non-dynamic VRAM.
bdf1ffc to
caedb3e
Compare
…Org#14480) * main: implement --vram-headroom Implement --vram-headroom for dynamic vram as a hybrid debug/diagnostic option that can be used for people who still report shared VRAM spills. They can trial and error the setting to maintain a bit more headroom to avoid shared VRAM spills. * main: implement --reserve-vram Implement --reserve-vram as extra headroom on the simple method which is semantically as close as possible to the stated functionality and formet behaviour of non-dynamic VRAM.
#14396
comfy-aimdo 0.4.10 implements:
1: double sync on VBAR free operations. There are still hard-to-reproduce observable de-syncs between the cuda observed free memory and the real actuals. Syncing after the fact may help and this should be reasonably cheap given that is was already synced before the free operation.
2: Implement two VRAM headroom configuration mechanisms. --reserve-vram by popular demand, and a more dynamic form --vram-headroom.
Example Test Conditions:
Linux, 5090, 96GB, LTX2.3
8GB VRAM held in use by a script
--reserve-vram 12
Before:
Comfy uses more than 32 - 12GB VRAM.
After:
The 12GB headroom is usable by the display driver and the 8GB background script so roughly 3GB is unused ✅
--vram-headroom 12(instead of--reserve-vram):The 12GB headroom is absolute ✅
Regression Tests:
Linux, 5090, 96GB, Ace-step turbo XL ✅
Linux, 5090, 96GB, stable cascade -> flux 2 ✅
Windows, 5060, 16GB, wan 2x14B FP8 ✅