14 KiB
14 KiB
Learnings
2026-04-17
- The workflow metadata already rides along in
extra_data.extra_pnginfo.workflow, so the backend can discovernode.properties.is_modal_remotewithout any extra frontend metadata channel. - A single wildcard-output proxy node is not enough for server-side graph rewriting. ComfyUI validates downstream link indices and output arity against the replacement class, so the backend has to generate proxy node classes that mirror the original node’s output signature before validation runs.
- Keeping the sync engine content-addressable makes prompt rewrites cheap after the first upload. The route can block on sync safely as long as repeated requests collapse to a hash marker check.
- Hashing and zipping the entire
custom_nodes/tree inside the queue route is too expensive forCOMFY_MODAL_EXECUTION_MODE=local. Local mode should skip custom-node bundle sync by default, with an explicit override when that path needs to be exercised. - When rewriting a prompt node into a proxy, never reuse the same input dict for both
prompt_node["inputs"]andoriginal_node_data["inputs"]. Insertingoriginal_node_databack into the live prompt inputs creates a recursive structure that breaks ComfyUI cache signature hashing with aRecursionError. - Queue-time validation is necessary for remote nodes that still receive local Comfy runtime objects. Even if a node is marked remote, inputs like
MODELorCONDITIONINGcannot be serialized across the current boundary, so the prompt should be rejected before execution rather than failing inside the serializer. - Per-node proxying is not enough for real ComfyUI sampler graphs. Connected remote-marked nodes must be partitioned into a single remote component so internal edges like
MODEL,CLIP, andCONDITIONINGstay inside the remote subgraph and only the true local/remote boundary has to be transport-safe. - Manually marking every upstream dependency is too brittle for arbitrary custom nodes. A better default is to auto-expand the remote node set upstream across any non-transportable edge, so a user can mark the downstream custom node they care about and let the backend absorb the required loaders/builders into the same remote island automatically.
- The local fallback runtime cannot call
PromptExecutor.execute()inline from inside a proxy node, because ComfyUI is already running inside an active asyncio loop. Running remote subgraph execution in a worker thread avoids nested-event-loop failures while still reusing ComfyUI’s executor. - Modal objects defined in-process are not directly callable from an arbitrary long-running app like ComfyUI unless they are inside a hydrated
app.run()session or looked up from a deployed app withmodal.Cls.from_name(...). In practice the correct default for this project is deployed lookup only, because ephemeralapp.run()fallback rebuilds too much state between workflow executions and undermines the scale-to-zero cost model. - A deployed-only strategy is still correct, but the custom node should bootstrap that deployment itself on first use. The pragmatic flow is: try
Cls.from_name(...), and if the app is missing, programmatically callapp.deploy(...)from the stable cloud entry module, then retry the lookup. - ComfyUI loads custom node packs from directories under synthetic module names derived from filesystem paths, and for directory-based packs that module key is not guaranteed to be a valid Python import path. Modal container startup requires a real importable module name for
@app.clsservices, so the cloud entrypoint has to live in a separate, stable module such ascomfyui_modal_sync_cloud.pyrather than on the custom-node package module itself. - Shipping the ComfyUI source tree into the Modal image is not enough by itself. The container also needs ComfyUI's import-time Python dependencies such as
psutil,aiohttp,einops,torchsde,torchvision, and related runtime packages, or the remote worker fails before it can execute any graph nodes. - Core ComfyUI imports extend beyond the obvious tensor stack. In practice the Modal image also needs startup/runtime packages such as
alembic,pydantic-settings,spandrel, andkornia, because built-in nodes and config-parsing paths import them before a workflow gets very far. - Quantized ComfyUI models add another hard dependency: if
comfy-kitchenis missing in the Modal image,comfy.quant_ops.get_layout_class(...)falls back toNoneand FP8/FP4 model loading eventually fails with'NoneType' object has no attribute 'Params'. - A captured "working workflow" exported after a remote run is not necessarily a pristine authoring graph. It can contain internal rewrite artifacts like
ModalUniversalExecutor, so checked-in workflow JSON should be treated explicitly as a smoke fixture unless it has been normalized. - When performance work starts, timestamped logging matters more than adding yet more ad hoc messages. Giving the extension its own formatter with both wall-clock time and
relativeCreatedmakes it much easier to see whether the delay is in hashing, sync, deployment, image build, or actual execution. - Once the slow path is visible, the first easy win is persistent local caching of digests and bundle artifacts. For this project, caching file hashes by
(path, size, mtime_ns)and caching digest-keyed custom_nodes archives under the local storage root avoids re-reading huge model files and re-zipping the same custom_nodes tree on every execution. - After the local side is fast, the next blind spot is inside the Modal container. The cloud runtime needs its own phase-timed logs around volume reload, runtime initialization, PromptExecutor execution, and output collection; otherwise the remote call just looks like one giant wall-clock block and there is no way to tell scheduling delay from real inference time.
- A dedicated Python logger is not always enough for those cloud timings. In practice, Modal container logs may still omit named-logger
INFOlines during ComfyUI startup, so the timing tracer should mirror its phase lines directly to container stdout to guarantee they appear in captured remote logs. - ComfyUI root discovery for deploy-time image building cannot rely only on
~/git/ComfyUI. When the extension is installed underComfyUI/custom_nodes/ComfyUI-Modal, the most reliable default is to derive the ComfyUI root from that install path and only fall back to home-directory heuristics. - If the remote image also contains ComfyUI's top-level
custom_nodes/tree, the container will import the full custom node set twice: once from the image and once from the extracted sync bundle. The image should ship ComfyUI core withoutcustom_nodes/, and the synced bundle should be the single source of external nodes. - ComfyUI still assumes that its default
custom_nodesdirectory exists even if you do not want to populate it from the image. If that directory is missing,nodes.init_external_custom_nodes()can crash onos.listdir(...), so the remote runtime should create an empty placeholder directory before external-node initialization. - Warm-container optimization is most effective when expensive but graph-agnostic work moves into
@modal.enter. For this project that includes built-in ComfyUI runtime initialization and execution-module imports, so snapshots and scaledown-window reuse can amortize that startup cost across requests. - On GPU-backed functions that only use CPU memory snapshots by default,
@modal.enter(snap=True)must stay CUDA-safe. Snapshot-phase prewarm should only do CPU-only work there, and the heavier ComfyUI runtime initialization should move to@modal.enter(snap=False)unless GPU snapshots are explicitly enabled. - If GPU memory snapshots are enabled, the snapshot-phase hook can safely pre-initialize more of ComfyUI core and capture that state. This repo now uses that split so the same code path benefits from either CPU-only snapshots or GPU snapshots without accidentally initializing CUDA in the wrong phase.
- Once warm-container runtime init is amortized, the next bottleneck is usually the graph's own loader nodes. For this repo,
UNETLoader,CLIPLoader, andVAELoaderare worth caching inside the warm container by resolved loader inputs, and cached outputs should be cloned when possible so later requests can reuse loaded runtime objects safely. - Loader caches are only part of the story. Local ComfyUI keeps one long-lived
PromptExecutorin the prompt worker, so ordinary node outputs stay cached across runs as long as their signatures do not change. A remote worker that creates a freshPromptExecutorper request throws that benefit away, so it needs a warm-container executor pool keyed by cache scope to preserve ComfyUI-style node-output reuse between runs. - End-to-end
prompt_executor_executetiming is still too coarse once the denoising loop is only a small fraction of the total. A headless prompt server can observeexecutingandexecutedevents fromPromptExecutorand emit per-node spans, which exposes whether the real time is in model loaders, encoders, or the sampler itself. - For repeat ComfyUI workflows, the highest-leverage lifecycle knob is usually not
min_containers=1but a longerscaledown_windowwithmin_containers=0. That lets a recently used container keep its fully initialized ComfyUI process warm for a while without forcing permanent idle billing between totally unrelated runs. - Floating
pip install torch torchvisionin the Modal image is too unstable once ComfyUI tracks very new PyTorch releases. The remote image should pin the same known-good CUDA wheel family as the local ComfyUI environment, and for this repo that means the explicit PyTorch CUDA 12.8 index withtorch==2.10.0,torchvision==0.25.0, andtorchaudio==2.10.0. - The frontend can reuse ComfyUI's native
executing,execution_error, andexecution_successwebsocket events for remote component visuals as long as the queue route also publishes the representative-node-to-original-node mapping after rewrite. That keeps the UI in sync with actual proxy execution instead of inventing a separate polling channel. - When filtering the ComfyUI checkout for the Modal image, only exclude top-level runtime asset directories like
models/andoutput/. A blanket "ignore any path part named models" rule will also delete real Python packages such ascomfy/ldm/models, causing import failures inside the container. - Importing
nodes.pyalone is not enough inside the Modal container. The remote worker also has to runnodes.init_extra_nodes(...)once so built-incomfy_extrasnodes such asEmptyFlux2LatentImageexist inNODE_CLASS_MAPPINGS, and then runinit_external_custom_nodes()for each extracted custom-node bundle root it exposes. - A bare
PromptExecutor(_NullPromptServer())is not equivalent to real ComfyUI startup. The headless worker must pass the same cache configuration thatmain.pyuses, or execution eventually crashes when the executor polls caches withself.cache_args["ram"]. - If
/modal/queue_promptdoes expensive preparatory work before it returns, the frontend can keep ComfyUI's own queue/execution indicators alive by dispatching temporary syntheticstatus,execution_start, andexecutingevents, then letting the real websocket events take over once the prompt is actually queued. - For remote node-level UI feedback, a streamed Modal generator method is cleaner than a side-channel file or volume poller. Yielding
progressenvelopes directly from the remote execution method lets the local client forward active-node updates into ComfyUI's websocket stream in real time without extra storage commits, reloads, or cleanup logic. - The synthetic frontend events have to match ComfyUI's websocket adapter shapes exactly, not just semantically. In practice that means
statusdetail must be the inner status payload,executingdetail should be the display node id string, and queue-list initialization state benefits from a matching"Waiting for a machine"notification payload keyed by prompt id. - Even with correctly-shaped synthetic events, ComfyUI's built-in queue UI may still not present an obvious "workflow running" state during long pre-queue Modal setup. The extension should own a separate global activity badge so users have explicit feedback while the remote runtime is still being prepared.
- Synced asset paths cannot stay as abstract
/assets/...markers inside the Modal worker. Remote execution needs those markers materialized to real absolute files under the mounted storage root, and ComfyUI'sfolder_pathshelpers need an absolute-path fast path so loader nodes can still resolve them. - A local filesystem mirror is not enough in
COMFY_MODAL_EXECUTION_MODE=remote. Queue-time sync must upload assets into the actual named Modal volume via the local SDK, and the remote worker must callvol.reload()before execution so warm containers can see newly uploaded files. - Once warm-container caches keep models and executors alive, unconditional
vol.reload()becomes wrong: Modal can reject reloads while files from the mounted volume are still open. The queue side already knows whether a request actually uploaded anything new, so that fact should travel in the payload and only requests with fresh uploads should reload the mounted volume. - Modal volume SDK calls should not run directly on the aiohttp request event loop. Running them in a worker thread avoids
AsyncUsageWarning, and a missing path fromVolume.listdir()should be treated as an ordinary "not uploaded yet" cache miss instead of a hard failure. - The Modal container can pick up a non-package
utilsmodule before ComfyUI initializes. Preloading ComfyUI's own top-levelutilspackage from the bundled source tree preventsutils.install_utilimport failures duringinit_extra_nodes(...). - If the goal is "no idle billing, but no minute-long rebuild every run", the right Modal pattern is a deployed app with
min_containers=0, not ephemeralapp.run(). That lets Modal reuse the built image and deployed objects while still scaling compute down to zero between executions. - Modal hardware selection belongs in deployment configuration, not in request payloads. For this repo, the remote class should read a configurable GPU string from environment at deploy time, and changing it later requires a redeploy or app recreation because existing deployed app hardware does not update in place.
- Modal memory snapshots only help once the app is deployed. They are a good default for CPU-side initialization work, while GPU memory snapshots should remain opt-in because Modal still labels them alpha and they may require additional runtime refactors.