Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Reduce repeated calls of get_immediate_node_signature in cache #4871

Merged
merged 1 commit into from
Sep 13, 2024
Merged

Reduce repeated calls of get_immediate_node_signature in cache #4871

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions comfy_execution/caching.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ def __init__(self, dynprompt, node_ids, is_changed_cache):
super().__init__(dynprompt, node_ids, is_changed_cache)
self.dynprompt = dynprompt
self.is_changed_cache = is_changed_cache
self.immediate_node_signature = {}
Copy link
Contributor

Choose a reason for hiding this comment

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

We can't persist the signatures in the cache keyed by ID. The whole point of the cache is that it persists between executions and there's no guarantee that node signatures will be the same from one execution to the next.

Long term, I think the right solution is to cache entire node definitions somewhere. I have that change locally, but it's grouped with a bunch of others and I won't be able to split it out until this weekend.

Shorter term, if you just reset the .immediate_node_signature at the beginning of each execution, that should work.

Copy link

Choose a reason for hiding this comment

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

My current hot fix right now is caching I/O calls within folder_paths.py (i.e os.path.*), which the I/O cache can reset whenever object_info api are called and it seems to work so far.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We can't persist the signatures in the cache keyed by ID. The whole point of the cache is that it persists between executions and there's no guarantee that node signatures will be the same from one execution to the next.

Long term, I think the right solution is to cache entire node definitions somewhere. I have that change locally, but it's grouped with a bunch of others and I won't be able to split it out until this weekend.

Shorter term, if you just reset the .immediate_node_signature at the beginning of each execution, that should work.

Currently, cache.set_prompt will reinitialize a cache_key_set each time see.

That is, the cache_key_set between each execution is two independent objects. So from the current implementation, .immediate_node_signature is indeed reset for each execution, because the cache_key_set is different.

This variable is added only to avoid repeated get_immediate_node_signature in get_node_signature of one workflow execution, and has nothing to do with other executions.

Of course, it would be better if this call could be further reduced, because the current implementation can only reduce one cache to be executed at a time, and there are also repeated get_immediate_node_signature between caches.outputs and caches.ui.

Copy link
Contributor Author

@JettHu JettHu Sep 12, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

My current hot fix right now is caching I/O calls within folder_paths.py (i.e os.path.*), which the I/O cache can reset whenever object_info api are called and it seems to work so far.

This is also a good approach. This PR takes into account more time-consuming INPUT_TYPES issues. I think I/O calls within folder_paths.py is just one of the exposed.

self.add_keys(node_ids)

def include_node_id_in_input(self) -> bool:
Expand Down Expand Up @@ -94,6 +95,8 @@ def get_immediate_node_signature(self, dynprompt, node_id, ancestor_order_mappin
if not dynprompt.has_node(node_id):
# This node doesn't exist -- we can't cache it.
return [float("NaN")]
if node_id in self.immediate_node_signature: # reduce repeated calls of ancestors
return self.immediate_node_signature[node_id]
node = dynprompt.get_node(node_id)
class_type = node["class_type"]
class_def = nodes.NODE_CLASS_MAPPINGS[class_type]
Expand All @@ -108,6 +111,7 @@ def get_immediate_node_signature(self, dynprompt, node_id, ancestor_order_mappin
signature.append((key,("ANCESTOR", ancestor_index, ancestor_socket)))
else:
signature.append((key, inputs[key]))
self.immediate_node_signature[node_id] = signature
return signature

# This function returns a list of all ancestors of the given node. The order of the list is
Expand Down
Loading