Commit Graph
259 Commits
Author SHA1 Message Date
John Pollock a8a5a6f1fd feat(__init__): add WEB_DIRECTORY constant for web assets path
Add a new constant WEB_DIRECTORY set to "./web" to define the directory path for web-related assets during package initialization. This improves organization by centralizing the path configuration. Additionally, removed trailing newline at file end to maintain consistent code formatting.
2025-09-30 17:02:59 -05:00
John Pollock 64d8ede091 prep for final release candidate 2025-09-30 10:41:14 -05:00
John Pollock b87505399f WIP 2025-09-30 10:38:16 -05:00
John Pollock 486a84e357 prep for final release candidate 2025-09-30 09:52:00 -05:00
John Pollock 62752d1bbf Standardize doc strings and make PEP 257 compliant 2025-09-30 09:34:41 -05:00
John Pollock fc2a732419 prepare for final release candidate 2025-09-30 09:15:32 -05:00
John Pollock 9a526e2546 prepare for final release candidate 2025-09-30 09:10:37 -05:00
John Pollock 23ed34df1b prepare for final release candidate 2025-09-30 09:08:15 -05:00
John Pollock 429be7c912 docs: update activeContext for v2.5.0 release with refactoring summary
Update active context documentation to reflect v2.5.0 release candidate status.
Major achievements documented:

- DisTorch2 allocation refactoring (-179 lines, unified UNET/CLIP logic)
- Production cleanup removing debug instrumentation (-40 lines)
- Verified selective unload system working with production logs
- Architecture status showing all core files production-ready
- Updated memory management pipeline with verification details

Reorganized content to prioritize recent session achievements (2025-09-30)
and production readiness status. Total code reduction: 219 lines through
consolidation and cleanup while maintaining full functionality.
2025-09-30 09:07:05 -05:00
John Pollock e7d8113a86 refactored in to one analyze_safetensor_loading 2025-09-30 08:44:16 -05:00
John Pollock 8b8a16e982 Major architectural refactor: Consolidate wrappers, fix CheckpointLoader bug, improve separation of concerns (-531 lines)
This commit represents a significant architectural refactoring to improve code organization,
eliminate redundancy, and fix a critical bug in wrapper functions. Net reduction of 531 lines
while improving maintainability and fixing functionality.

## wrappers.py (NEW FILE: +531 lines)
- Created dedicated module for ALL node wrapper/override functions
- Consolidated 10 wrapper types from 3 different files into single location:
  * DisTorch V2 SafeTensor wrappers (factory + 3 implementations)
  * DisTorch V1 legacy wrappers (4 GGUF/CLIP wrappers, rewritten to call V2 backend)
  * Standard MultiGPU wrappers (3 device selection wrappers)
- CRITICAL FIX: All wrappers now strip MultiGPU-specific parameters before calling
  original ComfyUI functions (fixes CheckpointLoaderSimple TypeError)
- Improved architecture: clear separation between wrapper UI and backend logic

## distorch.py (DELETED: -529 lines)
- Removed entire legacy DisTorch V1 file
- All V1 wrapper functions moved to wrappers.py and rewritten to call V2 backend
- Backend allocation functions no longer needed (V2 backend handles all cases)
- Eliminates code duplication and maintenance burden

## distorch_2.py (-409 lines)
- Removed duplicate _create_distorch_safetensor_v2_override factory function
  (was incorrectly present in both distorch_2.py and wrappers.py)
- Removed 3 wrapper export functions (moved to wrappers.py)
- File now contains ONLY backend logic:
  * register_patched_safetensor_modelpatcher()
  * analyze_safetensor_loading() and analyze_safetensor_loading_clip()
  * calculate_safetensor_vvram_allocation()
  * Allocation stores and model hash functions
- Added clear documentation comment about wrapper migration

## __init__.py (-230 lines)
- Removed 3 local wrapper function definitions (moved to wrappers.py)
- Removed soft_empty_cache_distorch2_patched (moved to device_utils.py)
- Removed all distorch.py imports (file deleted)
- Added imports from new wrappers.py module (10 wrapper functions)
- Updated imports from distorch_2.py (backend functions only, no wrappers)
- Improved architecture: __init__.py now focused on initialization and registration

## device_utils.py (+68 lines)
- Moved soft_empty_cache_distorch2_patched() from __init__.py
- Added comprehensive memory management patch in architecturally correct location
- Patch includes:
  * DisTorch2 detection and multi-device VRAM management
  * Adaptive CPU memory threshold checking
  * Force flag support for executor cache reset (Manager parity)
- Applied patch at module level: mm.soft_empty_cache = soft_empty_cache_distorch2_patched
- Behavior preserved: patch still executes when device_utils is imported by __init__.py

## nodes.py (-30 lines)
- Removed unused wrapper function imports
- Cleaned up import statements to reflect new architecture

## Impact Summary
- Improved architecture: Clear separation between wrappers (UI) and backend (logic)
- Eliminated distorch.py: Reduced from 3 files to 2 (wrappers.py + distorch_2.py)
- Net code reduction: 531 lines removed while adding functionality
- Better maintainability: Single source of truth for all wrapper functions
- Preserved behavior: All patches execute correctly, no functional changes

## Breaking Changes
None - this is a pure refactor with no API or behavioral changes.
2025-09-30 08:13:21 -05:00
John Pollock 07b429f3f9 fix(distorch): Add GC anchor protection for selective model retention
Problem: Models correctly categorized as "keep loaded" during selective
unload were disappearing before the next cleanup cycle. After reassigning
mm.current_loaded_models = kept_models, Python's garbage collector would
clear the models because the list was their only remaining strong reference.

Solution: Implement GC anchor system using a global set to hold strong
references to ModelPatcher objects that must survive cleanup cycles.

Changes:
- Add _MGPU_RETENTION_ANCHORS global set and helper functions
- Add early delegation check: if no DisTorch models want unload, clear
  anchors and delegate to original unload_all_models
- Add retention anchor when categorizing kept models
- Clear anchors before delegating to allow normal cleanup

Result: Self-contained, reversible protection mechanism. Models with
keep_loaded=True survive automatic cleanup but can be cleared with
explicit "Clear All Models" button. Tested on both keep_loaded=True
and keep_loaded=False scenarios.

Refs: memory-bank/distorch_selective_unload_solution.md
2025-09-29 16:01:24 -05:00
John Pollock bde51c6236 docs(distorch): document GC anchor solution for selective unload
Add comprehensive documentation for the DisTorch selective model unload
solution that addresses Python garbage collection issues. The document
details:

- Problem: Models with keep_loaded=False were being prematurely garbage
  collected despite being added to current_loaded_models list
- Root cause: Reassigning current_loaded_models created the only strong
  reference, making models vulnerable to GC between assignment and next
  access
- Solution: Global GC anchor set (_MGPU_RETENTION_ANCHORS) maintains
  strong references to ModelPatcher objects that should survive cleanup
- Implementation: Early delegation check, anchor protection during
  categorization, and explicit lifecycle management
- Testing results: Confirms Flux unloads while VAE/CLIP remain protected

This solution ensures DisTorch models can selectively unload while
keeping VAE and CLIP models loaded, preventing memory management race
conditions with Python's garbage collector.
2025-09-29 15:45:46 -05:00
John Pollock c23dc083d3 WIP 2025-09-29 14:04:05 -05:00
John Pollock 6e1f9671f2 docs(memory): update Phase 3 implementation status and clarify bug
Update documentation to reflect that Phase 3 selective ejection is now
fully deployed in code, including per-model flags, patched unload_all_models,
and Manager parity via force_full_system_cleanup.

Clarify understanding of remaining bug:
- Remove incorrect hypothesis that "all-kept delegation" causes the issue
- Document that delegation to original unload_all_models is intentional
  behavior needed for post-execution cleanup
- Simplify bug description to acknowledge root cause is still unknown
- Focus on observable symptom: retained models (keep_loaded=True) are
  still being ejected despite selective unload logic being present

This commit corrects misleading documentation and removes false leads
to help focus investigation on the actual unknown root cause.
2025-09-29 13:21:57 -05:00
John Pollock 01df082651 docs(memory-bank): sync with current code state for selective ejection\n\n- Document Phase 3 implemented without global sentinel (_mgpu_unload_distorch_model per-model flag)\n- Describe patched unload_all_models selective behavior and current all-kept delegation caveat\n- Outline rediscovery plan and strict no-op target when no models are flagged\n- Update active context, system patterns, code references, progress, tech context, and lineage 2025-09-29 07:57:35 -05:00
John Pollock 1ca3daf0d8 At least now the logs reflect it is now trying to do what I know we have figured out how to do in the past in one of these commits. . . 2025-09-29 07:08:03 -05:00
John Pollock d61ca7b06f back to setting full reset flag if there is a distorch unload pending. 2025-09-29 05:27:53 -05:00
John Pollock ede0957f65 feat: add caching and logging to IS_CHANGED methods in safetensor overrides
- Implement caching of the last computed settings hash using a class attribute `_last_hash`
- Compare current hash against the cached one to detect changes
- Add logging to indicate first call or when settings have changed, using shortened hash for brevity
- Applied consistently across `override_class_with_distorch_safetensor_v2`, `_v2_clip`, and `_v2_clip_no_device`
- Improves efficiency by avoiding redundant change detection and aids debugging of settings modifications
2025-09-29 04:46:07 -05:00
John Pollock c4ae5e9e08 extensive clean-up, WIP 2025-09-29 03:53:21 -05:00
John Pollock 23d2abe237 fixed incorrect info 2025-09-28 19:19:20 -05:00
John Pollock 8591063a3c incremental progress (I think, hard to tell) 2025-09-28 19:14:47 -05:00
John Pollock 18493f5277 refactor: simplify model retention logic in multi-GPU unload
- Renamed `keep_loaded` variable to `should_retain` for improved clarity
- Simplified assignment by directly retrieving `_mgpu_keep_loaded` attribute with default False
- Updated logging accordingly; may alter behavior for non-DisTorch models to no longer retain automatically
2025-09-28 13:22:19 -05:00
John Pollock 0d056141c0 an interesting experiment that produces wrong behavior but no OOM. Looks like we are circling it and I don't want to lose this intermediate step. 2025-09-28 12:15:52 -05:00
John Pollock ae8bb7cf2c feat: Refine model retention logic in multi-GPU unloading
- Modified condition to retain models lacking `_mgpu_keep_loaded` attribute or with `keep_loaded=True`
- Improves reliability of unloading by distinguishing DisTorch and non-DisTorch models
- Addresses potential premature unloading of intended persistent models in multi-GPU setups
2025-09-28 11:14:34 -05:00
John Pollock fda5d6ed00 commiting this steaming pile of hot garbage for future dissection to see if I want any organs from this terminally ill branch 2025-09-25 14:36:13 -05:00
John Pollock bd672479fa refactor: eliminate circular import by separating model management functions
- Create model_management_mgpu.py for centralized model lifecycle tracking
- Move memory management functions from device_utils.py to new module:
  * multigpu_memory_log, track_modelpatcher, trigger_executor_cache_reset
  * check_cpu_memory_threshold, prune_distorch_stores, try_malloc_trim
  * force_full_system_cleanup
- Update imports across codebase (distorch_2.py, distorch.py, __init__.py,
  nodes.py, checkpoint_multigpu.py)
- Resolves device_utils.py ↔ distorch_2.py circular dependency
- Follows established clean coding patterns with fail-fast error handling

Addresses critical CPU memory leak investigation infrastructure by ensuring
proper module separation for comprehensive memory management utilities.
2025-09-24 17:38:15 -05:00
John Pollock ff6efb4217 Scorched Earth, but it works.
feat: add configurable multi-GPU memory cleanup policies

- Add MULTIGPU_CLEANUP_POLICY environment variable with options: off, threshold, every_load, every_load+threshold
- Add MULTIGPU_CPU_RESET_THRESHOLD for memory threshold-based cleanup (default 0.85)
- Add MULTIGPU_MALLOC_TRIM toggle to control malloc trimming behavior
- Implement cleanup triggers in load_models_gpu based on configured policy
- Make malloc trim conditional in soft_empty_cache_distorch2_patched
- Add configuration logging for better observability

This allows users to customize memory management behavior for multi-GPU setups through
2025-09-24 13:55:31 -05:00
John Pollock 7b319544e0 feat: implement comprehensive memory management and OOM prevention
- Add ModelPatcher lifecycle tracking with weakref-based cleanup
- Implement reference cycle fixes in LoadedModel to prevent memory leaks
- Add memory threshold monitoring and automatic cleanup triggers
- Enable multigpu memory logging for debugging (MGPU_MM_LOG=True)
- Add OOM handling with graceful cleanup and recovery mechanisms
- Import additional memory utilities for cache management and malloc trimming
2025-09-23 22:52:20 -05:00
John Pollock cd7a536645 docs: add development rules and project context in .clinerules
Establish comprehensive development guidelines including project overview,
memory bank documentation requirements, technical patterns, and current
status for ComfyUI-MultiGPU contributors. Includes critical CPU memory
leak investigation details and mandated development philosophy.
2025-09-23 20:58:03 -05:00
John Pollock 3121b2f70c feat(mgpu): scoped MM logger; parse compute device/VRAM plan
- Introduce MGPU_MM_LOG flag and logger.mgpu_mm_log(...) to gate and
  prefix MultiGPU Model Management logs (disabled by default)
- Replace ad-hoc logger.info("[MultiGPU ...]") calls with mgpu_mm_log
  in DisTorch2 cache-clearing and delegation paths to reduce noise
- In load_models_gpu, parse safetensor allocation strings to infer
  incoming_compute_device and incoming_compute_planned_bytes (supports
  hash#device;GB and expert fraction syntax); track required bytes
- Remove coarse large-model threshold heuristic in favor of allocation-
  informed planning

Why: centralize and quiet verbose MGPU logs by default, and enable
smarter, data-driven device selection and memory planning for multi-GPU
model loading.
2025-09-23 04:41:44 -05:00
John Pollock a0fe72e290 Additonal refinements to DisTorch2 cache/unload to avoid OOM. Needs at least one more clean-up pass.
- Introduce MEMORY_LOG flag and logger.memory method to gate high-volume memory logs
- Demote device setter logs from info to debug to reduce noise
- Clarify patch announcement (remove text_encoder_initial_device mention)
- Update soft_empty_cache patch log to emphasize multi-device allocation/clearing; delegate to original when DisTorch2 is inactive
- Rework load_models_gpu preflight for large DisTorch2 models:
  - more robust ModelPatcher detection (direct or via .patcher)
  - track allowed devices and incoming model names
  - improved large-model detection and proactive unload/clearing on donor/offload devices
  - mitigates OOM during large model (e.g., UNet) swaps
- Minor cleanup of verbose comments and wording in logs
2025-09-21 09:30:03 -05:00
John Pollock 55a0d22b01 refactor: simplify memory logging in checkpoint loading
Replace try-catch wrapped comfyui_memory_load calls with streamlined
multigpu_memory_log function. Removes exception handling overhead and
uses consistent config hash identifiers for UNet, VAE, and CLIP model
loading phases.
2025-09-21 06:12:20 -05:00
John Pollock 63ff1a4064 committing so we don't lose verbose logging.
- Add comfyui_memory_load and create_model_identifier utilities (device_utils)
- Log GPU memory before/after UNet, VAE, and CLIP construction and after UNet weight load
- Include model identifiers in logs to correlate memory to specific patchers
- Guard logging calls with try/except to avoid impacting load flow
- Improves observability of memory usage for multi-GPU checkpoints and aids OOM/debugging
2025-09-20 11:58:29 -05:00
John Pollock 8e4c7fed14 Potential improvement - committing for additional testing
multi-GPU cache clear + proactive unload to prevent OOM

- Patch mm.soft_empty_cache to clear caches on all GPUs when DisTorch2 models are active; otherwise delegate to original ComfyUI behavior. Uses safetensor allocation store and model hashes to detect DisTorch2 models; adds soft_empty_cache_multigpu import.
- Patch mm.load_models_gpu (guarded to apply once) to proactively unload large, unneeded models (>2GB) before loading large DisTorch2 models. Frees compute and donor device memory to prevent UNet OOM during model swaps.
- Preserve original functions for fallback, validate inputs, and log clearly to reduce risk during reloads and unexpected usage.
2025-09-20 07:08:45 -05:00
John Pollock 57c7d3da8e Merge pull request #109 from pollockjj/low_vram_clip
Fix #104: Remove text_encoder_initial_device patch, simplify text encoder device handling, bump to 2.4.7
2025-09-15 13:22:40 -05:00
John Pollock f7942dca93 Fix for (#104): drop text_encoder_initial_device patch and state - these were part of an attempt to solve a CLIP compute issue that was recently solved another way (Commit edc8a4d)
- Remove current_text_encoder_initial_device and its updates
- Delete text_encoder_initial_device_patched and stop overriding mm.text_encoder_initial_device
- Simplify set_current_text_encoder_device and logging to track only current_text_encoder_device

Bump revision to 2.4.7
2025-09-15 12:46:02 -05:00
John Pollock 80f8a14dea Fix non-deterministic behavior of CLIP compute device when ~100% offloading. 2025-09-14 14:59:01 -05:00
John Pollock aa00a682d0 feat: add model inspection utilities for tracking and analysis
- Extend module docstring to include inspection capabilities
- Add create_model_identifier() to generate unique hashes from model type and size
- Add analyze_tensor_locations() to analyze tensor device placement and memory usage
- Include imports for hashlib, psutil, and comfy.model_management to support new features

These utilities enable end-to-end tracking of model state and placement for better debugging and management in multi-GPU setups.
2025-09-14 00:22:28 -05:00
John Pollock edc8a4dd2b Identified a long-standing bug where fully-allocated CLIP (for example 99G of VirtualVRAM = 100% of major blocks no matter the model) proceeded to execute on the donor device (e.g. cpu) instead of the indicated compute device. Turns out, it only happens when *all* blocks are identified to go onto the donor card. In the case of the donor being the cpu this was irritatingly slow.
On a 4x PCIe bus, swapping a normal CLIP-sized number of layers once/twice (for neg) into compute should be the optimal solution:  Reside on `cpu`, use the optimized cuda kernals for computation JiT on `compute`, discard layers once used (residing permenantly on `cpu`), then move efficently to the main UNet computation.
2025-09-14 00:07:47 -05:00
John Pollock d34a32f097 Fix for Triple/Quad Clip Loaders (#99) 2025-09-12 23:44:20 -05:00
John Pollock afafc8042d Add no-device variants for multi-GPU CLIP loaders 2025-09-12 22:39:50 -05:00
John Pollock 5bb7add514 Remove unused 'device' parameter from CLIP loader methods
This parameter was not utilized in the load_clip methods of TripleCLIPLoaderGGUF
and QuadrupleCLIPLoaderGGUF, so it has been removed to eliminate run-time errors.
2025-09-12 22:18:26 -05:00
John Pollock fabbc9e7be Merge branch 'lora_reapply_fix' 2025-09-10 19:44:08 -05:00
John Pollock 5b62671f0c roll back aggresive memory management 2025-09-10 19:17:05 -05:00
John Pollock e9fb4a8c2f Hot FixL: Revert aggresive memory management until a more targeted approach can be developed. This was causing OOMs on models that should load normally using the normal loader.
Update version to 2.4.4.
2025-09-10 18:24:12 -05:00
John Pollock be9cc21d4d preliminary changes 2025-09-10 14:55:02 -05:00
John Pollock e1635e9996 Improve memory handling for safetensor models in corner cases
- Added preemptive model unloading and cache clearing in register_patched_safetensor_modelpatcher() to resolve potential memory issues when allocations are unavailable, prompting the usage of the standard loaders.
2025-09-09 12:00:56 -05:00
John Pollock 803cf542d9 MultiGPU garbage collection/cache clearing and DisTorch2 Clip device node bug 2025-09-08 23:10:54 -05:00
John Pollock c63b539f1e Additional garbage/cache collection (#101) addressed DisTorch2 Device issue for CLIP hopefully closing (#99,#104)
Add comprehensive memory cache clearing aligned with ComfyUI patterns to improve stability and reduce OOM incidents in multi-device scenarios.

**Addresses Memory/Garbage Collection Issues:**
- Created `soft_empty_cache_multigpu()` function in device_utils.py
- Replicates ComfyUI's cache clearing for all devices (CUDA, MPS, XPU, NPU, MLU)
- Includes CUDA IPC collect optimization like ComfyUI
- Strategically placed calls before major memory allocations

**Addresses CLIP loading issues:**
- Fixed DisTorch2 device device varibale management before text encoder operations

**`soft_empty_cache_multigpu()` implementation Aligned with ComfyUI's Patterns:**
- Called after GC operations
- Placed before major memory allocations
- Matches ComfyUI's proven memory management strategy
- Same device clearing logic for multi-device scenarios
2025-09-08 23:06:21 -05:00