Skip to content

Load weights to process RAM with MRU policy using pinning infrastructure#15027

Draft
rattus128 wants to merge 4 commits into
Comfy-Org:masterfrom
rattus128:prs/hard-loading
Draft

Load weights to process RAM with MRU policy using pinning infrastructure#15027
rattus128 wants to merge 4 commits into
Comfy-Org:masterfrom
rattus128:prs/hard-loading

Conversation

@rattus128

@rattus128 rattus128 commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

#14618

Currently comfy relies on the OS disk cache to cache model weights that are loaded directly to the GPU (and not offloaded). There are a number of reports of this being unreliable.

This change brings the caching logic in-house to comfy with a lower priority set of pinned weights for loaded (as in not offloaded) weights. This achieves three goals:

1: Forcibly use RAM to load as much model as possible so for any weird OS decision to de-prioritize comfy RAM, we now have the RAM greedily as actual allocation

2: Bring into play pinning of the primary model load. Users with lots of RAM doing multi-model flows should be able to benefit from having the models pinned in ram each workflow RAM and get it onto the GPU with no RAM->RAM copy at all

3: Evict RAM on a watermark level rather than LRU. This avoids the scenario where the user has to reload the entire workflow from disk if their RAM is less than their needs and all the eviction schemes are LRU and it just evicts everything in cycles.

The cost is an extra RAM->RAM copy in the first place which makes it a little slower on the first workflow run in particular. It also hogs all RAM for the process which means means no consolidation of caching across comfy restarts or multiple instances of comfy.

Use --fast-disk if you want to knock this out and rely on disk performance to minimize/share RAM. --fast-disk is slightly different with its pinning behaviours relative to current no args, but its far closer than this change. We can make the old way a separate option but its probably best to just consolidate with this and --fast-disk.

The retention policy now changes to retaining older models over new ones while still deprioritizing inactive workflow models. This avoids the LRU problem, but at the cost of sometimes retaining an un-needed model over a needed one when considering caching behaviors. The classic case is same prompt with sampler seed change, where you want to keep the diffusion model more than the TE. It will however learn after another run as the TE will get kicked out as an inactive model. So it can take an extra workflow run to reach steady state after prompt change. There isnt really a good solution to this as we simply dont have future knowledge on what the next workflow is going to do. But the reasonable best assumption is the user is going to rerun everything exactly as you just did ignoring any cachables. So MRU evicition / watermark system is preferable to avoid repeat runners having LRU cascade failure. This will regress second run performance in particular for some usage points, but the steady state takes priority. See below in the "after" with the 3rd run being faster than anything else.

Example test conditions:

RTX5090, 32GB, Linux, WAN 2x14B FP16, PCIE1 x4 NVME (downgraded from PCIE5)

image

This case used to lose everything to the LRU problem. Also because Linux it fully pins what it retains so extra boost on the rerun speed.

Before (2 runs):

[INFO] got prompt
[INFO] Using pytorch attention in VAE
[INFO] Using pytorch attention in VAE
[INFO] VAE load device: cuda:0, offload device: cpu, dtype: torch.bfloat16
[INFO] Found quantization metadata version 1
[INFO] Using MixedPrecisionOps for text encoder
[INFO] CLIP/text encoder model load device: cuda:0, offload device: cpu, current: cpu, dtype: torch.float16
[INFO] Requested to load WanTEModel
[INFO] Model WanTEModel prepared for dynamic VRAM loading. 6419MB Staged. 0 patches attached. Force pre-loaded 73 weights: 488 KB.
[INFO] Model WanTEModel prepared for dynamic VRAM loading. 6419MB Staged. 0 patches attached. Force pre-loaded 73 weights: 488 KB.
[INFO] Requested to load WanVAE
[INFO] Model WanVAE prepared for dynamic VRAM loading. 241MB Staged. 0 patches attached. Force pre-loaded 60 weights: 61 KB.
[INFO] model weight dtype torch.float16, manual cast: None
[INFO] model_type FLOW
[INFO] Requested to load WAN21
[INFO] Model WAN21 prepared for dynamic VRAM loading. 27251MB Staged. 400 patches attached. Force pre-loaded 160 weights: 1603 KB.
100%|██████████| 2/2 [00:15<00:00,  7.86s/it]                                   
[INFO] model weight dtype torch.float16, manual cast: None
[INFO] model_type FLOW
[INFO] Requested to load WAN21
[INFO] 0 models unloaded.
[INFO] Model WAN21 prepared for dynamic VRAM loading. 27251MB Staged. 400 patches attached. Force pre-loaded 160 weights: 1603 KB.
100%|██████████| 2/2 [00:14<00:00,  7.19s/it]                                   
[INFO] 0 models unloaded.
[INFO] Model WanVAE prepared for dynamic VRAM loading. 241MB Staged. 0 patches attached. Force pre-loaded 60 weights: 61 KB.
[INFO] Prompt executed in 103.51 seconds
[INFO] got prompt
[INFO] Model WAN21 prepared for dynamic VRAM loading. 27251MB Staged. 400 patches attached. Force pre-loaded 160 weights: 1603 KB.
100%|██████████| 2/2 [00:13<00:00,  6.58s/it]                                   
[INFO] 0 models unloaded.
[INFO] Model WAN21 prepared for dynamic VRAM loading. 27251MB Staged. 400 patches attached. Force pre-loaded 160 weights: 1603 KB.
100%|██████████| 2/2 [00:06<00:00,  3.22s/it]                                   
[INFO] Model WanVAE prepared for dynamic VRAM loading. 241MB Staged. 0 patches attached. Force pre-loaded 60 weights: 61 KB.
[INFO] Prompt executed in 88.36 seconds

End of run state (2 runs):

image

After:

[INFO] got prompt
[INFO] Using pytorch attention in VAE
[INFO] Using pytorch attention in VAE
[INFO] VAE load device: cuda:0, offload device: cpu, dtype: torch.bfloat16
[INFO] Found quantization metadata version 1
[INFO] Using MixedPrecisionOps for text encoder
[INFO] CLIP/text encoder model load device: cuda:0, offload device: cpu, current: cpu, dtype: torch.float16
[INFO] Requested to load WanTEModel
[INFO] Model WanTEModel prepared for dynamic VRAM loading. 6419MB Staged. 0 patches attached. Force pre-loaded 73 weights: 488 KB.
[INFO] Model WanTEModel prepared for dynamic VRAM loading. 6419MB Staged. 0 patches attached. Force pre-loaded 73 weights: 488 KB.
[INFO] Requested to load WanVAE
[INFO] Model WanVAE prepared for dynamic VRAM loading. 241MB Staged. 0 patches attached. Force pre-loaded 60 weights: 61 KB.
[INFO] model weight dtype torch.float16, manual cast: None
[INFO] model_type FLOW
[INFO] Requested to load WAN21
[INFO] Model WAN21 prepared for dynamic VRAM loading. 27251MB Staged. 400 patches attached. Force pre-loaded 160 weights: 1603 KB.
100%|██████████| 2/2 [00:17<00:00,  8.59s/it]                                   
[INFO] model weight dtype torch.float16, manual cast: None
[INFO] model_type FLOW
[INFO] Requested to load WAN21
[INFO] 0 models unloaded.
[INFO] Model WAN21 prepared for dynamic VRAM loading. 27251MB Staged. 400 patches attached. Force pre-loaded 160 weights: 1603 KB.
100%|██████████| 2/2 [00:08<00:00,  4.20s/it]                                   
[INFO] 0 models unloaded.
[INFO] Model WanVAE prepared for dynamic VRAM loading. 241MB Staged. 0 patches attached. Force pre-loaded 60 weights: 61 KB.
[INFO] Prompt executed in 106.85 seconds
[INFO] got prompt
[INFO] Model WAN21 prepared for dynamic VRAM loading. 27251MB Staged. 400 patches attached. Force pre-loaded 160 weights: 1603 KB.
100%|██████████| 2/2 [00:06<00:00,  3.21s/it]                                   
[INFO] 0 models unloaded.
[INFO] Model WAN21 prepared for dynamic VRAM loading. 27251MB Staged. 400 patches attached. Force pre-loaded 160 weights: 1603 KB.
100%|██████████| 2/2 [00:06<00:00,  3.22s/it]                                   
[INFO] Model WanVAE prepared for dynamic VRAM loading. 241MB Staged. 0 patches attached. Force pre-loaded 60 weights: 61 KB.
[INFO] Prompt executed in 80.87 seconds
[INFO] got prompt
[INFO] Model WAN21 prepared for dynamic VRAM loading. 27251MB Staged. 400 patches attached. Force pre-loaded 160 weights: 1603 KB.
100%|██████████| 2/2 [00:13<00:00,  6.54s/it]                                   
[INFO] 0 models unloaded.
[INFO] Model WAN21 prepared for dynamic VRAM loading. 27251MB Staged. 400 patches attached. Force pre-loaded 160 weights: 1603 KB.
100%|██████████| 2/2 [00:14<00:00,  7.01s/it]                                   
[INFO] 0 models unloaded.
[INFO] Model WanVAE prepared for dynamic VRAM loading. 241MB Staged. 0 patches attached. Force pre-loaded 60 weights: 61 KB.
[INFO] Prompt executed in 74.72 seconds

Load state after (3 runs)

image

Performance / Regression Tests (3 runs of each WF - time in seconds):

Windows, RTX3060, 32GB RAM, PCIE1 x4 NVME

Workflow    comfy/master                  This change
SD1.5       6.644 /   2.531 /   2.518     6.629 /   2.540 /   2.519
SDXL       22.994 /  13.764 /  13.798    23.397 /  13.740 /  13.788
Wan FP8   114.228 / 102.461 / 102.294   121.956 / 105.311 /  98.747
Wan FP16  147.798 / 130.049 / 134.715   151.782 / 137.637 / 140.645
Flux 2    307.268 / 239.772 / 239.617   309.525 / 243.040 / 244.548
Z-Image    39.546 /  20.320 /  20.371    37.201 /  20.300 /  20.373

Linux, RTX5090, 32GB RAM, PCIE1 x4 NVME

Workflow    comfy/master                This Change
SD1.5       3.464 /  0.453 /  0.444     3.663 /  0.455 /  0.442
SDXL        9.767 /  1.960 /  1.958    10.753 /  1.953 /  1.957
Wan FP8    48.917 /  9.814 /  8.586    52.187 /  8.543 /  8.550
Wan FP16   73.427 / 71.949 / 71.961    79.596 / 53.223 / 53.983
Flux 2     77.886 / 16.225 / 15.508    82.292 / 16.746 / 17.216
Z-Image    18.787 /  2.636 /  2.642    19.775 /  2.640 /  2.641

@github-actions

github-actions Bot commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

✅ All contributors have signed the CLA. Thank you! This PR is ready to be merged.
Posted by the CLA Assistant Lite bot.

@rattus128

Copy link
Copy Markdown
Contributor Author

I have read and agree to the Contributor License Agreement

comfy-legal added a commit to Comfy-Org/comfy-cla that referenced this pull request Jul 22, 2026
@rattus128 rattus128 changed the title Load weights to process RAM with MRU policy using pinning infrastrcture Load weights to process RAM with MRU policy using pinning infrastructure Jul 22, 2026
@rattus128
rattus128 marked this pull request as ready for review July 24, 2026 04:57
@coderabbitai

coderabbitai Bot commented Jul 24, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

The update introduces loaded and non-loaded pin subsets for dynamic models, stores pin metadata per module and subset, and refactors budget, registration, eviction, reset, and RAM accounting paths. Casting selects between corresponding weight and patch subsets. Execution cache links now track source sockets and invoke callbacks, allowing prompt execution to mark dynamic model patchers in use and clear them at prompt boundaries. RAM pressure handling also recognizes wrapped model patcher outputs.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 2.44% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: loading weights into process RAM with MRU-style pinning and eviction.
Description check ✅ Passed The description directly explains the pinning, RAM caching, and MRU eviction changes described by the patch.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

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.

❤️ Share

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

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 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/ops.py`:
- Around line 205-210: The patches pin-selection logic around get_pin and
handle_pin should reuse an existing "patches-loaded" pin when the initial
"patches" lookup returns None. Check "patches-loaded" before applying the
signature/fast-disk fallback, preserving the existing final "patches" behavior
when no loaded pin is available.

In `@execution.py`:
- Around line 668-705: Move model-patcher discovery and prompt-pin state
management out of execution.py into the model-management/pinning owner behind an
explicit wrapper/classification contract. In execution.py lines 668-705 remove
_mark_current_prompt_models, _clear_current_prompt_models, and related
ModelPatcher state handling; at line 769 invoke the owner’s prompt-start
lifecycle, at line 812 pass its opaque classification callback, and at line 875
invoke the owner’s prompt-end lifecycle. In comfy_execution/caching.py line 570
use that owner-provided classification contract instead of probing
output.patcher with getattr; update execution.py lines 668-705, 769, 812, and
875 and comfy_execution/caching.py line 570 accordingly, ensuring parent control
flow performs no child getattr probing.
🪄 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: ASSERTIVE

Plan: Pro Plus

Run ID: b4eef493-24f2-47e1-b93b-98ff8151764e

📥 Commits

Reviewing files that changed from the base of the PR and between f6d30bc and 824ad8a.

📒 Files selected for processing (7)
  • comfy/model_management.py
  • comfy/model_patcher.py
  • comfy/ops.py
  • comfy/pinned_memory.py
  • comfy_execution/caching.py
  • comfy_execution/graph.py
  • execution.py
📜 Review details
🧰 Additional context used
📓 Path-based instructions (7)
**/*

📄 CodeRabbit inference engine (AGENTS.md)

**/*: Keep changes small, direct, and limited to the narrowest code path and fewest files necessary; prefer practical fixes over broad architectural work.
Prefer fewer dependencies and do not add a ComfyUI dependency unless absolutely necessary.
Remove obsolete code, dead branches, unused options, debug prints, and unnecessary compatibility paths.
Preserve existing APIs, node names, model-loading behavior, file layout, and workflow compatibility unless replacement is explicit.
Keep architectural layers focused; do not leak UI, API, workflow, queue, persistence, telemetry, model-loading, node, or execution concerns into unrelated layers.
Shared core modules should depend only on lower-level primitives and their own domain concepts; higher-level concepts belong at callers, adapters, services, or boundaries.
Pass only the narrowest data needed across boundaries and keep identity mapping, persistence, history, telemetry, response shaping, and UI state in their owning layers.
Before touching many files, identify the smallest owner layer; use caller-side mappings, adapters, events, or narrow interfaces instead of exposing private concepts across layers.
Core ComfyUI code must not make unsolicited internet requests or add uploads, telemetry, analytics, tracking, reporting, update checks, remote configuration, licensing checks, or similar outbound paths.
Model downloading is allowed only when explicitly authorized by the user, limited to the requested artifact, and free of telemetry, tracking, unrelated metadata, or background activity.
Warning and info messages should be short and actionable; documentation and README changes should be concise, factual, and tied to changed behavior.
Use short direct commit subjects such as Fix ..., Add ..., Support ..., Remove ..., or Update ...; keep PR descriptions short and state the problem, behavior change, and tests.
Prefer one coherent behavioral change per commit and prioritize crashes, incorrect dtype/device behavior, m...

Files:

  • comfy_execution/caching.py
  • comfy/ops.py
  • comfy_execution/graph.py
  • execution.py
  • comfy/pinned_memory.py
  • comfy/model_patcher.py
  • comfy/model_management.py
**/*.py

📄 CodeRabbit inference engine (AGENTS.md)

**/*.py: Keep state and capability flags on the object that owns the behavior; use explicit parent-owned fields rather than probing children with getattr for parent control flow.
Keep public methods aligned with caller contracts; preserve arguments, parameter order, return types, side effects, and error behavior unless all affected interfaces are intentionally updated.
Do not add unused compatibility parameters, flags, attributes, constructor options, or model-specific options to shared helpers.
Normalize third-party and upstream return conventions at integration boundaries so core code receives the expected type and shape.
Avoid caller-side unwrapping such as out = out[0] unless the called interface documents that return structure.
Do not add torch.no_grad, torch.inference_mode, or inference-mode wrappers; only disable a globally enabled inference mode when a training path requires gradients.
Do not add freeze, unfreeze, or trainability toggles to ComfyUI model classes.
Remove training-only behavior such as dropout from inference models while preserving checkpoint and state-dict compatibility, using nn.Identity when necessary to retain slots.
Keep imports at module scope; use inline imports only for established optional-backend probes or import-cycle avoidance.
Use try/except only for optional dependency, platform, or backend detection with a useful fallback, and prefer specific exception types.
Do not add code for unsupported pinned library versions or obsolete PyTorch workarounds; unsupported formats, quantization metadata, and bad states should fail clearly.
Match local file style and keep comments sparse, useful, and non-obvious; remove comments that merely restate code.

Files:

  • comfy_execution/caching.py
  • comfy/ops.py
  • comfy_execution/graph.py
  • execution.py
  • comfy/pinned_memory.py
  • comfy/model_patcher.py
  • comfy/model_management.py
**

⚙️ CodeRabbit configuration file

**: IMPORTANT: Only comment on issues directly introduced by this PR's code changes.
Treat AGENTS.md as mandatory repository policy, not optional style guidance.
Flag PR changes that violate AGENTS.md even when the code is otherwise functional.
In particular, enforce architecture boundaries, dtype/device/memory rules,
interface contracts, import style, no unnecessary try/except blocks, no inline
imports, no outbound internet paths in core ComfyUI, and narrow scoped fixes.
Prefer direct findings over suggestions when a rule is violated. Only ignore
AGENTS.md when it clearly conflicts with a newer explicit maintainer instruction
in the PR.
Do NOT flag pre-existing issues in code that was merely moved, re-indented,
de-indented, or reformatted without logic changes. If code appears in the diff
only due to whitespace or structural reformatting (e.g., removing a with: block),
treat it as unchanged. Contributors should not feel obligated to address
pre-existing issues outside the scope of their contribution.

Files:

  • comfy_execution/caching.py
  • comfy/ops.py
  • comfy_execution/graph.py
  • execution.py
  • comfy/pinned_memory.py
  • comfy/model_patcher.py
  • comfy/model_management.py
comfy_execution/**

⚙️ CodeRabbit configuration file

comfy_execution/**: Execution engine (graph execution, caching, jobs). Focus on:

  • Caching correctness
  • Concurrent execution safety
  • Graph validation edge cases

Files:

  • comfy_execution/caching.py
  • comfy_execution/graph.py
comfy/**/*.py

📄 CodeRabbit inference engine (AGENTS.md)

comfy/**/*.py: Treat dtype, device placement, VRAM use, offloading, and memory behavior as correctness concerns; use existing ComfyUI quantization, memory, offload, and optimized-operation helpers.
Prefer shared optimized kernels, backend dispatchers, and documented interfaces over duplicate handwritten operations; treat selected backend callables as opaque.
Do not duplicate operations with custom float32-upcasting inference kernels; use generic ComfyUI operations or native PyTorch operations.
If a model constructor has an operations parameter, assume it is never None; do not add fallback torch operations.
Avoid unnecessary parameters in model, block, and operation constructors or forwards; reuse existing model classes, blocks, operations, and helpers.
Model detectors must inspect only the first dimension of linear weights, guard every dereferenced state-dict key, and order specific signatures before broad fallbacks.
Avoid einops in core inference code; use native tensor operations such as reshape, view, permute, transpose, flatten, and related methods.
Keep metadata, counters, shape calculations, indices, split boundaries, and control-flow values as Python values rather than tensors.
Avoid unnecessary casts and transfers; preserve intended compute and storage dtypes, tensor shapes, and backend result contracts.
Keep model-native latent layout handling inside the model or latent-format owner rather than reshaping it in nodes or caller-side adapters.
DiT models must pad every patchified target or reference input with comfy.ldm.common_dit.pad_to_patch_size and crop only the target output back to its original dimensions.
Do not add defensive shape, configuration, or dtype casts that merely obscure a clear tensor-operation failure; validate only at meaningful boundaries.
Raw parameters not owned by an operation should be cast at use with comfy.ops.cast_to_input or comfy.model_management.cast_to; model constructors should not contain dtype workaro...

Files:

  • comfy/ops.py
  • comfy/pinned_memory.py
  • comfy/model_patcher.py
  • comfy/model_management.py
comfy/**

⚙️ CodeRabbit configuration file

comfy/**: Core ML/diffusion engine. Focus on:

  • Backward compatibility (breaking changes affect all custom nodes)
  • Memory management and GPU resource handling
  • Performance implications in hot paths
  • Thread safety for concurrent execution

Files:

  • comfy/ops.py
  • comfy/pinned_memory.py
  • comfy/model_patcher.py
  • comfy/model_management.py
execution.py

📄 CodeRabbit inference engine (AGENTS.md)

execution.py should consume the prompt graph and execution state, produce results and errors, and remain unaware of workflow, frontend, persistence, and API-only identifiers.

Files:

  • execution.py
🧠 Learnings (2)
📚 Learning: 2026-02-21T14:01:41.482Z
Learnt from: pythongosssss
Repo: Comfy-Org/ComfyUI PR: 12555
File: comfy_extras/nodes_glsl.py:719-724
Timestamp: 2026-02-21T14:01:41.482Z
Learning: In PyOpenGL, bare Python scalars can be accepted for 1-element array parameters by NumberHandler. This means you can pass an int/float directly to OpenGL texture deletion (e.g., glDeleteTextures(tex)) without wrapping in a list. Verify function-specific expectations and ensure types match what the OpenGL call expects; use explicit lists only when the API requires an array.

Applied to files:

  • comfy_execution/caching.py
  • comfy/ops.py
  • comfy_execution/graph.py
  • execution.py
  • comfy/pinned_memory.py
  • comfy/model_patcher.py
  • comfy/model_management.py
📚 Learning: 2026-05-13T12:31:45.069Z
Learnt from: rattus128
Repo: Comfy-Org/ComfyUI PR: 13802
File: comfy/pinned_memory.py:19-30
Timestamp: 2026-05-13T12:31:45.069Z
Learning: When reviewing code that uses comfy/pinned_memory.py’s `HostBuffer.extend(size=..., reallocate=...)`: by default (`reallocate` is not True / False), `extend(size=...)` is a *relative increment* that grows the buffer by `size` bytes—so slicing like `[offset:offset+size]` after `hostbuf.extend(size=size)` is correct and the argument should not be rewritten to `offset + size`. Only in the single-segment reallocation mode (`reallocate=True`, e.g., as used by `resize_pin_buffer()` in `comfy/model_management.py`) should `size` be treated as an *absolute target* and the call/arguments should be checked accordingly.

Applied to files:

  • comfy/ops.py
  • comfy/pinned_memory.py
  • comfy/model_patcher.py
  • comfy/model_management.py
🪛 ast-grep (0.44.1)
execution.py

[warning] 764-875: Do not use an empty list as a default parameter
Context: async def execute_async(self, prompt, prompt_id, extra_data={}, execute_outputs=[]):
set_preview_method(extra_data.get("preview_method"))

    nodes.interrupt_processing(False)
    self._clear_current_prompt_models()

    if "client_id" in extra_data:
        self.server.client_id = extra_data["client_id"]
    else:
        self.server.client_id = None

    self.status_messages = []
    self.add_message("execution_start", { "prompt_id": prompt_id}, broadcast=False)

    self._notify_prompt_lifecycle("start", prompt_id)
    ram_headroom = int(self.cache_args["ram"] * (1024 ** 3))
    ram_inactive_headroom = int(self.cache_args["ram_inactive"] * (1024 ** 3))
    ram_release_callback = self.caches.outputs.ram_release if self.cache_type == CacheType.RAM_PRESSURE else None
    comfy.memory_management.set_ram_cache_release_state(ram_release_callback, ram_headroom)

    try:
        with torch.inference_mode():
            dynamic_prompt = DynamicPrompt(prompt)
            reset_progress_state(prompt_id, dynamic_prompt)
            add_progress_handler(WebUIProgressHandler(self.server))
            is_changed_cache = IsChangedCache(prompt_id, dynamic_prompt, self.caches.outputs)
            for cache in self.caches.all:
                await cache.set_prompt(dynamic_prompt, prompt.keys(), is_changed_cache)
                cache.clean_unused()

            node_ids = list(prompt.keys())
            cache_results = await asyncio.gather(
                *(self.caches.outputs.get(node_id) for node_id in node_ids)
            )
            cached_nodes = [
                node_id for node_id, result in zip(node_ids, cache_results)
                if result is not None
            ]

            comfy.model_management.cleanup_models_gc()
            self.add_message("execution_cached",
                          { "nodes": cached_nodes, "prompt_id": prompt_id},
                          broadcast=False)
            pending_subgraph_results = {}
            pending_async_nodes = {} # TODO - Unify this with pending_subgraph_results
            ui_node_outputs = {}
            executed = set()
            execution_list = ExecutionList(dynamic_prompt, self.caches.outputs, self._mark_current_prompt_models)
            current_outputs = self.caches.outputs.all_node_ids()
            for node_id in list(execute_outputs):
                execution_list.add_node(node_id)

            while not execution_list.is_empty():
                node_id, error, ex = await execution_list.stage_node_execution()
                if error is not None:
                    self.handle_execution_error(prompt_id, dynamic_prompt.original_prompt, current_outputs, executed, error, ex)
                    break

                assert node_id is not None, "Node ID should not be None at this point"
                result, error, ex = await execute(self.server, dynamic_prompt, self.caches, node_id, extra_data, executed, prompt_id, execution_list, pending_subgraph_results, pending_async_nodes, ui_node_outputs)
                self.success = result != ExecutionResult.FAILURE
                if result == ExecutionResult.FAILURE:
                    self.handle_execution_error(prompt_id, dynamic_prompt.original_prompt, current_outputs, executed, error, ex)
                    break
                elif result == ExecutionResult.PENDING:
                    execution_list.unstage_node_execution()
                else: # result == ExecutionResult.SUCCESS:
                    execution_list.complete_node_execution()

                if self.cache_type == CacheType.RAM_PRESSURE:
                    ram_release_callback(ram_inactive_headroom)
                    ram_shortfall = ram_headroom - psutil.virtual_memory().available
                    if ram_shortfall > 0:
                        freed = ram_release_callback(ram_headroom, free_active=True, min_entry_size=RAM_CACHE_LARGE_INTERMEDIATE)
                        ram_shortfall -= freed
                    if comfy.model_management.should_free_pins_for_ram_pressure(ram_shortfall):
                        freed = comfy.model_management.free_pins(ram_shortfall + 512 * (1024 ** 2))
                        if freed < ram_shortfall:
                            if freed > 64 * (1024 ** 2):
                                # AIMDO MEM_DECOMMIT can outrun psutil.available catching up.
                                time.sleep(0.05)
                            ram_release_callback(ram_headroom, free_active=True)
            else:
                # Only execute when the while-loop ends without break
                # Send cached UI for intermediate output nodes that weren't executed
                for node_id in dynamic_prompt.all_node_ids():
                    if node_id in executed:
                        continue
                    if not _is_intermediate_output(dynamic_prompt, node_id):
                        continue
                    cached = await self.caches.outputs.get(node_id)
                    if cached is not None:
                        display_node_id = dynamic_prompt.get_display_node_id(node_id)
                        _send_cached_ui(self.server, node_id, display_node_id, cached, prompt_id, ui_node_outputs)
                self.add_message("execution_success", { "prompt_id": prompt_id }, broadcast=False)

            ui_outputs = {}
            meta_outputs = {}
            for node_id, ui_info in ui_node_outputs.items():
                ui_outputs[node_id] = ui_info["output"]
                meta_outputs[node_id] = ui_info["meta"]
            self.history_result = {
                "outputs": ui_outputs,
                "meta": meta_outputs,
            }
            self.server.last_node_id = None
            if comfy.model_management.DISABLE_SMART_MEMORY:
                comfy.model_management.unload_all_models()
    finally:
        comfy.memory_management.set_ram_cache_release_state(None, 0)
        self._clear_current_prompt_models()
        self._notify_prompt_lifecycle("end", prompt_id)

Note: [CWE-710] Improper Adherence to Coding Standards (mutable default argument).

(no-empty-list-as-parameter)

🔇 Additional comments (5)
comfy_execution/graph.py (1)

198-240: LGTM!

comfy/model_patcher.py (1)

1727-1740: LGTM!

Also applies to: 1811-1812, 1954-2011

comfy/pinned_memory.py (1)

12-17: LGTM!

Also applies to: 19-48, 50-69, 71-127

comfy/model_management.py (1)

635-678: LGTM!

Also applies to: 688-699, 701-713, 1403-1413

comfy/ops.py (1)

147-153: LGTM!

Also applies to: 186-195

Comment thread comfy/ops.py
Comment on lines +205 to +210
subset = "patches"
pin = comfy.pinned_memory.get_pin(lowvram_source, subset=subset)
if pin is None and signature is not None and not args.fast_disk:
subset = "patches-loaded"
pin = comfy.pinned_memory.get_pin(lowvram_source, subset=subset)
handle_pin(lowvram_source, pin, lowvram_source, lowvram_dest, subset=subset, size=lowvram_size)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
rg -n '_lowvram_function' --type=py -C3 comfy/

Repository: Comfy-Org/ComfyUI

Length of output: 3402


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== comfy/ops.py excerpt =="
sed -n '120,218p' comfy/ops.py | cat -n

echo
echo "== lowvram pin setup excerpts =="
sed -n '1760,1860p' comfy/model_patcher.py | cat -n

echo
echo "== pinned memory interface =="
fd -a pinned_memory.py . | sed 's#^\./##'
if [ -f comfy/pinned_memory.py ]; then
  wc -l comfy/pinned_memory.py
  sed -n '1,220p' comfy/pinned_memory.py | cat -n
fi

Repository: Comfy-Org/ComfyUI

Length of output: 16828


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== ops.py cast_maybe_lowvram_patch and prepare behavior =="
sed -n '168,220p' comfy/ops.py | cat -n

echo
echo "== LowVramPatch implementation =="
rg -n "class LowVramPatch|is_lowvram_patch|def prepare|def clear_prepared|def memory_required" --type=py comfy/ -C4

echo
echo "== model_patcher lowvram setup continuation =="
sed -n '1830,1885p' comfy/model_patcher.py | cat -n

echo
echo "== pinned_memory eviction/discard relevant methods =="
rg -n "def .*pin|ensure_pin_budget|free_registrations|registerable_size|balancer_priority|_pins|discard_cuda_async_error" comfy/pinned_memory.py comfy/model_management.py --type=py -C3

Repository: Comfy-Org/ComfyUI

Length of output: 38895


Keep the loaded pin when choosing the patches subset.

The weights path reuses "weights-loaded" whenever an existing unloaded pin is available. The patches path should do the same: if get_pin(..., subset="patches") returns None, check "patches-loaded" before falling only to "patches" on a signature, otherwise a leftover loaded patch pin is ignored and a new pinned allocation is made instead.

🤖 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/ops.py` around lines 205 - 210, The patches pin-selection logic around
get_pin and handle_pin should reuse an existing "patches-loaded" pin when the
initial "patches" lookup returns None. Check "patches-loaded" before applying
the signature/fast-disk fallback, preserving the existing final "patches"
behavior when no loaded pin is available.

Comment thread execution.py
Comment on lines +668 to +705
self.current_prompt_models = {}
self.reset()

def _mark_current_prompt_models(self, outputs):
if isinstance(outputs, Mapping):
outputs = outputs.values()
elif not isinstance(outputs, (list, tuple)):
outputs = (outputs,)

for output in outputs:
if isinstance(output, (Mapping, list, tuple)):
self._mark_current_prompt_models(output)
continue

models = []
if isinstance(output, ModelPatcher):
models.append(output)
models.extend(output.model_patches_models())
models.extend(output.get_nested_additional_models())
else:
patcher = getattr(output, "patcher", None)
if isinstance(patcher, ModelPatcher):
models.append(patcher)
get_models = getattr(output, "get_models", None)
if callable(get_models):
models.extend(get_models())

for model in models:
if not isinstance(model, ModelPatcher) or not model.is_dynamic():
continue
key = (id(model.model), model.load_device)
self.current_prompt_models[key] = model
model.set_in_use_by_current_prompt(True)

def _clear_current_prompt_models(self):
for model in self.current_prompt_models.values():
model.set_in_use_by_current_prompt(False)
self.current_prompt_models.clear()

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy lift

Keep model-patcher discovery in the pinning/model-management owner.

execution.py now imports, discovers, and mutates ModelPatcher state, while both layers probe arbitrary outputs with getattr. Move wrapper classification and prompt-pin lifecycle behind a narrow model-management/pinning boundary with an explicit wrapper contract.

  • execution.py#L668-L705: remove execution-owned ModelPatcher traversal and state tracking.
  • execution.py#L769-L769: invoke the owner’s prompt-start lifecycle instead.
  • execution.py#L812-L812: pass an owner-provided opaque callback.
  • execution.py#L875-L875: invoke the owner’s prompt-end lifecycle instead.
  • comfy_execution/caching.py#L570-L570: use the same owner-provided classification instead of probing .patcher.

As per coding guidelines, execution.py must remain unaware of model-loading and parent control flow must not probe children with getattr; the path policy requires narrow layer boundaries.

📍 Affects 2 files
  • execution.py#L668-L705 (this comment)
  • execution.py#L769-L769
  • execution.py#L812-L812
  • execution.py#L875-L875
  • comfy_execution/caching.py#L570-L570
🤖 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 `@execution.py` around lines 668 - 705, Move model-patcher discovery and
prompt-pin state management out of execution.py into the
model-management/pinning owner behind an explicit wrapper/classification
contract. In execution.py lines 668-705 remove _mark_current_prompt_models,
_clear_current_prompt_models, and related ModelPatcher state handling; at line
769 invoke the owner’s prompt-start lifecycle, at line 812 pass its opaque
classification callback, and at line 875 invoke the owner’s prompt-end
lifecycle. In comfy_execution/caching.py line 570 use that owner-provided
classification contract instead of probing output.patcher with getattr; update
execution.py lines 668-705, 769, 812, and 875 and comfy_execution/caching.py
line 570 accordingly, ensuring parent control flow performs no child getattr
probing.

Sources: Coding guidelines, Path instructions

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