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.

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

ComfyUI Queue hangs when (re-)executing complex workflows #1502

Open
rgthree opened this issue Sep 13, 2023 · 7 comments
Open

ComfyUI Queue hangs when (re-)executing complex workflows #1502

rgthree opened this issue Sep 13, 2023 · 7 comments
Labels
Feature A new feature to add to ComfyUI.

Comments

@rgthree
Copy link
Contributor

rgthree commented Sep 13, 2023

I've noticed that ComfyUI queue execution hangs significantly when re-executing a previous prompt with many "pipe" or "context" nodes.

It looks like recursive_output_delete_if_changed in execution.py goes through upwards of hundreds of millions of recursions depending on the length of the workflow.

@rgthree
Copy link
Contributor Author

rgthree commented Sep 13, 2023

FWIW, since I'm deep in debugging here, I beleive recursive_output_delete_if_changed can be optimized. Can have a PR if interested.

rgthree added a commit to rgthree/ComfyUI that referenced this issue Sep 13, 2023
@rgthree rgthree changed the title ComfyUI Queue hangs when re-executing complex workflows ComfyUI Queue hangs when (re-)executing complex workflows Sep 13, 2023
@rgthree
Copy link
Contributor Author

rgthree commented Sep 13, 2023

Looks like recursive_will_execute could use the same optimization too. Added to same PR.

@JPS-GER
Copy link

JPS-GER commented Sep 18, 2023

I've experienced those slowdowns with my workflow.

It took 5+ minutes, if I just generated again with the same seed. Even generating a new seed from a fresh start took less time than generating the same seed again. Only fix was to change my workflow a lot by replacing pipes with reroutes (which triggered the lost connections problem - another problem rgthree fixed). During those 5+ minutes a single virtual CPU core was running at 100% - nothing else happend - no errors, no significant RAM/VRAM usage, etc.

If needed I can provide the "broken" workflow.

@rgthree
Copy link
Contributor Author

rgthree commented Sep 18, 2023

@JPS-GER I did patch this in rgthree-comfy so if you're using my nodes, it should be faster. I had only patched the recursive re-execute method first, but just pushed a change to patch the other one from the PR as well.

If you pull the latest https://github.com/rgthree/rgthree-comfy you should see this message in the terminal on startup:

[rgthree] Optimizing ComfyUI reursive execution. If queueing and/or re-queueing seems broken, change "patch_recursive_execution" to false in rgthree_config.json

Let me know if this helps your workflow. The impact of the patch for me should be the same as the pull request here (though, I'd rather it be in ComfyUI than just rgthree-comfy)

For a rather complex test workflow, the patch reduces iterations of recursive_will_execute from 113,292,566 to 135 (and 116.32 seconds to 69.84 seconds on my machine) on a fresh queue, and reduces recursive calls of recursive_output_delete_if_changed from 250,496,808 to 142 (and 158.13 seconds to 0.0 seconds on my machine).

@JaredTherriault
Copy link

This appears to change the output order in a way I don't expect. it should just patch the comfy behaviour, right? Where the output nodes with the fewest eval nodes in the chain get executed first? But I am finding the opposite happens, where ShowText and ShowPreview nodes get executed after the main image output, and this is not ideal when upscaling, interpolating, or using AnimateDiff.

@rgthree
Copy link
Contributor Author

rgthree commented Oct 4, 2023

@JaredTherriault good call; the PR did indeed change the order. Just committed 50b3fb1 fixes that.

The original code builds up in immense list, and the original patch cut that short. 50b3fb1 changes it from a list to a counter since the call site only cares about the len anyway.

Good news is its even faster than before, ha!

@JaredTherriault
Copy link

@rgthree thanks so much for the update, I really like your suite btw!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature A new feature to add to ComfyUI.
Projects
None yet
4 participants