Commit Graph
272 Commits
Author SHA1 Message Date
John Pollock 0919fd4ccb Minor cleanup related to 2.5.1 release 2025-10-04 18:08:22 -05:00
John Pollock f60aa6a9a7 refactor: keep_loaded --> eject_models Boolean switch. Use it to eject all other models prior to loading model for inference; helpful to maximize available latent space on device prior to UNet inference, for example
So this is a change from something just newly-released in 2.5.0, but most should either see an improvement or no change to behavior. This was the weakest, and jankiest part of 2.5.0 and my decision to manage a CPU memory leak turned into a too-aggressive solution with unwanted side effects.

This solution should provide a better way to manage `compute` VRAM as the most asked-for feature is a way to remove everything else from VRAM prior to main UNet inference, which this accomplishes nicely, as well as reporting back accurate information DisTorch2 on-device shard sizes.
2025-10-04 17:42:52 -05:00
John Pollock 72a20338ef feat: patch load_models_gpu for accurate memory calculations; unpatch load_models_gpu
Refactor memory management in distorch_2.py to patch load_models_gpu instead of LoadedModel.model_memory_required. Implement correct memory reporting based on model flags (eject_models and is_distorch_model), ensuring proper eviction logic and improved handling of virtual VRAM. This drives behavior purely by either comfy core matching or DisTorch flag, fixing potential issues in multi-GPU setups.
2025-10-04 17:16:51 -05:00
John Pollock e6d19951d7 Paranoia before cleanup 2025-10-04 12:32:37 -05:00
John Pollock e3750fd737 minor changes 2025-10-04 09:27:33 -05:00
John Pollock ee41f46beb revert most changes 2025-10-04 06:47:08 -05:00
John Pollock d8616acd5e investigation 2025-10-04 05:29:32 -05:00
John Pollock a1b7b1fdcf Merge pull request #114 from pollockjj/d2_clip
Major Refactor
2025-09-30 20:38:39 -05:00
John Pollock 2a6a8f4c2b workflow cleanup 2025-09-30 20:33:48 -05:00
John Pollock a19e31c915 corrected instructions 2025-09-30 18:38:08 -05:00
John Pollock 24b27c4c83 docs: add node documentation guide to README
Add comprehensive guide for accessing and using documentation on core MultiGPU and DisTorch2 nodes, covering 36+ nodes with detailed parameters, outputs, and usage examples. This enhances user experience by providing easy reference for standard ComfyUI loaders and DisTorch2 features, while clarifying coverage excludes third-party nodes.
2025-09-30 18:32:57 -05:00
John Pollock 989b3dc4c3 additional standard loaders documentation 2025-09-30 18:10:51 -05:00
John Pollock 7e0d484b17 docs: Add documentation for standard, gguf, and DisTorch nodes/wrappers 2025-09-30 18:05:48 -05:00
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