chrisgoringe / cg-use-everywhere

Apache License 2.0
355 stars 28 forks source link

Reporting a couple of issues with performance and autocomplete #146

Closed LukeG89 closed 3 months ago

LukeG89 commented 3 months ago

Hi, I want to report this couple of issues I found:

1) I've noticed a strange behavior with fps on the bottom-left corner.

This is with your node disabled:

fps_with_node_disabled

And this when enabled:

fps_with_node_enabled

It happens even if there is no Anything Everywhere nodes inside the workflow. Is your node "stressing" something?

I've also tried disabling all this settings, but nothing has changed. A_e_settings

2) This is a very specific one regarding autocomplete.

Yesterday I've merged together some Anything Everywhere? nodes with other custom nodes using Group Nodes, to tide up a complex workflow. When I was doing it, I've noticed that my CPU was getting stressed, even if I was just creating groups and nothing else.

Then, when putting "randomize" oncontrol_before_generate (before, not after), this happened to my seed:

https://github.com/chrisgoringe/cg-use-everywhere/assets/161304036/58258f1a-5216-40f7-96ae-2ab7c38257e3

I found out that it was for some reason the Autocomplete mode.

chrisgoringe commented 3 months ago

thanks - will investigate

JorgeR81 commented 3 months ago

About the FPS, I don't notice anything different ( but I'm always only at little below 60 ).

But I was able to reproduce the autocomplete issue.

On a side note,

It seems cg-use-everywhere, already partially breaks control_before_generate, even before autocomplete. It causes the seed to be changed every time you click on any node, which should not happen. I noticed that in the video, and I also have it.

It seems this issue always existed, since control_before_generate was added. https://github.com/comfyanonymous/ComfyUI/pull/2538#issuecomment-1907445906

JorgeR81 commented 3 months ago

@LukeG89, try [ Seed (rgthree) ] + [ Anything Everywhere? ]

For me it works very well. 

It shows the current seed ( last queued seed ), if I set it to random ( -1 ). And I can make the current seed "fixed", just by clicking the lower button.

rg

rg2

LukeG89 commented 3 months ago

About the FPS, I don't notice anything different ( but I'm always only at little below 60 ).

Did you notice any differences when not moving the mouse? I see that if I keep the mouse still when cg-use-everywhere is disabled, fps "stops" at a frame rate. When the node is enabled, fps just spins.

It causes the seed to be changed every time you click on any node, which should not happen.

Exactly, I was about to report it too. I thought it was a normal behavior because I had cg-use-everywhere already installed.

try [ seed (rgthree) ] + [ anything everywhere? ]

For me it works very well.

Thanks @JorgeR81! It seems this is the only seed that doesn't change when moving around nodes.

JorgeR81 commented 3 months ago

Seed (rgthree) is more convenient, for me overall.

But it still has an issue with the use case you described, with the grouped nodes.

It seems the current ( random ) seed is converted to "fixed" each time I queue a prompt. So I have to click to randomise it again, every time, before running a new queue.

rg3

JorgeR81 commented 3 months ago

Did you notice any differences when not moving the mouse? I see that if I keep the mouse still when cg-use-everywhere is disabled, fps "stops" at a frame rate. When the node is enabled, fps just spins.

No, I'm always at 5x.xx ~ 59.xx.  It's always changing slightly, very fast.  

fps

I only noticed drops in FPS, in very large workflows, with many UE nodes, when I used the new "pulse" animation, which uses blur in the UE wires.

chrisgoringe commented 3 months ago

Thanks for the report - I'll look into performance!

chrisgoringe commented 3 months ago

OK, the underlying issue here is with the frequency with which the canvas gets redrawn. If the redraw takes longer than 16ms, and has to happen every frame, the fps will drop below 60.

A change due to animation requires a redraw. Also, because of the way ComfyUI works, detecting if you need to animate only happens on a redraw (this might be solvable).

So for now I've made the following changes:

Take a look, see how much this helps.

JorgeR81 commented 3 months ago

Yes, these seem like good changes. I don't know what @LukeG89 has to say, but for me it was already working fine, so I didn't notice much difference. ( EDIT: I do notice a difference, but only if I look directly at FPS values, see the next comment. )

In my case, being at 60 FPS, it's normal, because I don't have a high frame rate monitor. I thought @LukeG89 issues were related to him having a high frame rate monitor. That's why I mentioned my case, and not having the issues he described.

About the FPS slowdown with "pulse", it's just related to the extra performance needed to draw the blur effect. I think it's normal. I have an old GPU. But it only becomes noticeable with a massive workflow, that is not a real use case. I just mentioned that as an extreme case. To prove that I'm not having performance issues during regular use.


But I do have a small issue related to UE links visualization in general, which was not solved by these latest changes. If there is a way to fix that, it would be great.

When I have UE visualization always on, if I mute or bypass a group or UE node, sometimes the wires are not all updated at the same time. For instance, here I just "muted" and "unmuted" this ControlNet group. But the wires for the negative conditioning were not redrawn. I need to click on a node for the missing wires to be redrawn.

test011

JorgeR81 commented 3 months ago

Actually, after the latest changes I do notice that the frame rate value changes at a slower pace, when the mouse it's not moving. The value changes only after 1 or 2 seconds, instead of changing every fraction of second. The value is still normally at about 59 ~ 60 FPS, but since it changes at a slower pace, I can actually read the full value, without using "print screen". I don't know if this is a good thing or a bad thing ... The GUI still feels responsive to me. Don't notice much difference.

Also, I never noticed the CPU and GPU spikes. But I have a big, silent case, so I don't have thermal issues, and I don't usually hear fans ramping up.

chrisgoringe commented 3 months ago

Interested to know if the fix I just pushed for another issue fixes the problem with links not updating.

JorgeR81 commented 3 months ago

Interested to know if the fix I just pushed for another issue fixes the problem with links not updating.

Yes, it does, it's fixed ! I noticed that the missing wires only show up about 1 or 2 seconds, after the other ones. But that's OK. 


But now the pale green UE wires are white ! It's better for them to be pale green, in order to not be confused with regular wires, from highlighted regular nodes. 

pl

JorgeR81 commented 3 months ago

By the way, I have one more question about animation performance:

The dot animation, for short UE wires, it's great. It's very smooth !

But for longer wires, it's too fast.

If that can be changed, I wouldn't mind losing a bit of performance to improve that.

LukeG89 commented 3 months ago

The value changes only after 1 or 2 seconds, instead of changing every fraction of second.

I don't know if this is a good thing or a bad thing ...

@JorgeR81 As you can see from my first post, the normal behavior of fps should be like that, UE is the cause of the frame rate instability.


@chrisgoringe Good improvement! However, I see that UE is still quite "heavy" to the system. Here's two comparisons: 1) Just starting ComfyUI with a simple workflow (no UE nodes used).

comparison_with-without_UE

2) Just opening the same complex workflow, but one with normal wires and the other with UE links.

comparison_with-without_UE_opening_workflow

In both cases, my CPU increases the frequency, using or not the node. Is it normal?

JorgeR81 commented 3 months ago

I see that UE is still quite "heavy" to the system

But is this with or without UE link animations ?

I think the latest performance improvements are more about UE animations, right ?

LukeG89 commented 3 months ago

But is this with or without UE link animations ?

With animation enabled (mouseover only option).

I just saw that disabling animation solve that CPU usage. But the strange part is having that high CPU usage even when there is no animations at all.

In my first example, there is just a workflow with literally two notes inside. And in the second case, I didn't trigger any animation with the mouse inside the workflow.

Do I have everytime to enable/disable animation in Settings?

JorgeR81 commented 3 months ago

Now there are several types of animations. Using "Dots" alone is better for performance. 

anim

There are also new ways to trigger UE link visualization. I think showing UE links only for "Selected nodes" would also be better for performance.

opt

LukeG89 commented 3 months ago

@JorgeR81 When testing I used one single animation (I don't remember which one) and selected the "Mouseover node" option.

However, overall now it is working better than before! I'm just wondering why there is that strange CPU/GPU usage even with no animation running 🤔

UE just checking if there is something to animate?

JorgeR81 commented 3 months ago

Another CPU usage issue ...

https://github.com/chrisgoringe/cg-use-everywhere/issues/152

I checked my task manager, to see if I also have this. I'm on Windows 10. I have an ancient 4 core ( 8 thread ) intel i7 7700 ( non K ) desktop CPU, from 2017.


larger worflow

wf

With a relative large UE workflow, I'm at about:

0 ~ 2 % CPU usage, with UE links visible, and animations disabled

5 ~ 10 % CPU usage, with dot animation enabled 

10 ~ 20 % CPU usage, with animation set to "both"

When I moved the workflow relatively fast, with "both" animation, I had a spike of about 85%. But only the first time I tried this. After that, it didn't spike again, on moving the workflow.

Later on, I had a spike of about 50 %, when changing from another chrome tab ( Gmail ) to ComfyUI. But also only one time. Usually it only goes from ~2% to ~20% and stays at about ~15 % when I switch tabs. All this with the "both" animation, worst case scenario.

The GUI was always responsive, even with the spikes. With my large case and CPU cooler I don't hear fans ramping up.  I only noticed something looking at the task manager. 

But, when moving this larger workflow, with "both" animation ( even when there are no spikes ), I don't have the instantaneous responsiveness, I get with animation disabled, or "dots" animation.


smaller workflow

wf2

With a smaller UE workflow, I'm at about:

5 ~ 6 % CPU usage, with animation set to "both"

With "both" animation I have spikes of about to 10 ~ 12 % ( from 1 ~ 2 % ) when switching from another tab ( Gmail ) to Comfy UI, and then it drops to about 5 ~ 6 %. And now I do have instantaneous ( or close to instantaneous ) responsiveness when I move this smaller workflow, even with "both" animation enabled.

( this smaller workflow, is the Stable Cascade img2img one I shared on a previous issue )

chrisgoringe commented 3 months ago

I'm loosing track here!

Created #153 to cover the CPU usage when UE isn't even doing anything.

Is there anything else specific?

LukeG89 commented 3 months ago

@chrisgoringe Now it seems to work really great! It's lighter!

Obviously when animation is running, even with minimum settings (dots and selected nodes), CPU and GPU starts cranking up like hell: in my case (gaming laptop) CPU usage goes from ~5% at ~2.40GHz to ~14% at ~3,7GHz and integrated GPU usage (not the NVidia one) increases from ~7% to ~30%. But at least now it's limited! Great improvement, good job! 🔥

JorgeR81 commented 3 months ago

For me, this was never really an issue. And if my old CPU can handle UE, the new ones should too. I think this was noticed, mostly by people with laptops, that can hear fans rampling up.

If someone else creates an issue about this, without being very specific ( like on the other issue ), maybe you could ask:

LukeG89 commented 3 months ago

And if my old CPU can handle UE, the new ones should too.

@JorgeR81 It was never about "handling it", but "why should my PC have to handle it?". UE is "just" an utility to have less wires inside the workflow, not a preprocessor or something that needs computing resources. This node should not affect CPU/GPU usages, it doesn't matter if it's a desktop or laptop.

chrisgoringe commented 3 months ago

I agree (with both of you...). There's an old idiom of software - "make it work, then make it useful, then make it fast". I'll always develop things at first without worrying about performance, but eventually the time comes for profiling and optimising!

Hmm. Thinking about @LukeG89's last comment - maybe there should be an option to turn all the animations etc off when the workflow is running... #156

JorgeR81 commented 3 months ago

The worst for performance is the blur required for the "pulse" animation.

But you just added "pulse", mostly to have an animation that works with "straight" wires, right ?!

If possible, maybe you could replace this with another type of "pulse".

If the UE wire transparency changes a bit, it can also create a sort of "pulse" effect.

If the transparency change is smooth, I think this would look even better than blur on the wires.

chrisgoringe commented 3 months ago

I'll look at other options (on my machine the dots are worse than the pulse...)

And I've just implemented #156 to turn animation off when the job is running.

JorgeR81 commented 3 months ago

on my machine the dots are worse than the pulse...

Really ??!! Must be my old hardware, that does not have an optimized way to handle blur ...


But anyways, another idea could be adding an option to identify UE wires, without animations.

e.g.:

A wire made with dashed lines ( not sure if possible ). A wire with slightly higher color saturation ( like you had before ). A wire with a bit of transparency ( but without the pulse effect ).

LukeG89 commented 3 months ago

@chrisgoringe, I just found a couple of things:

https://github.com/chrisgoringe/cg-use-everywhere/assets/161304036/62a77704-ee33-4173-8af1-57bef924eb48

chrisgoringe commented 3 months ago

Is the first point covered by https://github.com/chrisgoringe/cg-use-everywhere/issues/144 ?

Basically a group node input can't be to a UE node.

chrisgoringe commented 3 months ago

@JorgeR81 @LukeG89 really appreciate your input on this.

I'm going to close this issue thread because its become pretty general. I think I've added specific issues for most of the issues / suggestions raised in it.

Please raise new issues when you find problems or have suggestions! Ideally each issue thread will focus on a single issue :).

Alternatively, head to https://github.com/chrisgoringe/cg-use-everywhere/discussions and start a discussion thread!