Skip to content

Hold Diffusers pipelines in the engine's local object store - #76

Open
cjkindel wants to merge 6 commits into
experiment/v2-exec-depsfrom
experiment/v2-exec-deps-object-cache
Open

cjkindel wants to merge 6 commits into
experiment/v2-exec-depsfrom
experiment/v2-exec-deps-object-cache

Conversation

@cjkindel

@cjkindel cjkindel commented Oct 6, 2026 •

Copy link
Copy Markdown
Contributor

Stacked on #66, which moves Diffusers into a worker-v2 execution environment but keeps the library's own
ModelCache. This PR is the second half: built pipelines move out of that cache and into the engine's
local object store.

The object cache

The library kept its own module-global ModelCache. Built pipelines now live in the worker's local
object store through node.local_objects:

  • Releasing GPU memory became the engine's job. on_drop=clear_diffusion_pipeline runs on
    displacement, node delete, and worker teardown. Before, release happened only if a node reached
    remove_pipeline on the way past.
  • One pipeline stays resident, as before. Building releases the one already held, which is what
    ModelCache.get_or_build_pipeline did when it cleared the whole cache on a new config hash. Holding two
    models' worth of VRAM would be a different experience, not just a new environment. The derived-pipeline
    path keeps its base rather than cascading, since a derived pipeline shares the base's components and
    releasing them would move them to the CPU underneath the pipeline being built from them.
  • Keys are namespaced per library, so co-tenants cannot displace each other and ClearPipelineCacheNode
    empties only this library's.

The artifact still carries config_hash rather than becoming a cache reference: a reference is
resolve-or-raise, and the description is needed on the orchestrator too.

The builder's state override is removed. It reported the node UNRESOLVED when its pipeline was
missing from the cache, but node state is read on the orchestrator and the pipeline is held in the
worker, so it always answered "not cached" and re-ran the builder on every execution.
get_or_build_pipeline rebuilds on a miss in the process that holds the cache.

Verification

The diff against #66 is the object-store adoption only; #66 carries the execution-environment move and
the merges from main. make check and make test/unit pass.

@cjkindel

cjkindel commented Oct 6, 2026

Copy link
Copy Markdown
Contributor Author

@griptapeops review

@griptapeops

griptapeops Bot commented Oct 6, 2026 •

Copy link
Copy Markdown
Contributor

✅ GriptapeOps finished @cjkindel's request in 8m 20s

  • Read PR and diff
  • Verify findings
  • Post review and status

Review posted at 052247c.

@griptapeops griptapeops Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The move to local_objects is aimed right, and the one-resident policy matches what ModelCache did. 1 correctness finding on the new Release Pipeline node, plus 1 LLMism.

  • release_pipeline_node.py:49: releasing one pipeline breaks the other one in the base + ControlNet pair
  • latent_diffusion_pipeline_builder_node.py:90: a comment describing code that was removed

Comment thread modular_diffusion_nodes_library/nodes/release_pipeline_node.py Outdated
Comment thread modular_diffusion_nodes_library/nodes/latent_diffusion_pipeline_builder_node.py Outdated
With one pipeline resident at a time, the only moment two are held is a base and its ControlNet pipeline, which share components. Releasing one of them tears down the components the other still uses, so targeted release differed from Clear Pipeline Cache only where it broke the survivor. Clear Pipeline Cache covers freeing memory between steps.

Also drops the builder comment about the removed state override.
@cjkindel

cjkindel commented Oct 6, 2026

Copy link
Copy Markdown
Contributor Author

@griptapeops re-review

@griptapeops

griptapeops Bot commented Oct 6, 2026 •

Copy link
Copy Markdown
Contributor

✅ GriptapeOps finished @cjkindel's request in 1m 28s

  • Read prior findings
  • Check new commits
  • Post re-review + status

Review posted at a34e105.

@griptapeops griptapeops Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Both findings resolved. a34e105 removes the Release Pipeline node and the stale builder comment; the other commits since 052247c are merges from main via the base branch.

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