search

zombieyang / sd-ppp

Communicate between Photoshop and SD/SDForge/ComfyUI
MIT License
132 stars 6 forks source link

Convert http to socketio, simplify by removing push data from PS. #6

Closed tianlang0704 closed 4 months ago

tianlang0704 commented 4 months ago

According to #4 I converted the http communication to socketio. And I've removed the code that pushes data from PS to ComfyUI so it's simpler and clearer. But there are some problems I had encountered:

  1. socketio seem to be extremely slow on my PC. It sometimes took multiple seconds to receive a simple message.
  2. I can't seem to get a reliable disconnect event from the socketio server in python.
  3. try catch would be everywhere with socketio callback centric pattern, it'll be very hard to be simple and reliable at the same time.

I think I'll stay away from socketio on my fork, but anyway here's the convertion to socketio for ComfyUI front end.

zombieyang commented 4 months ago

Is the delay of socketio in your pc equals to the timeout option of socket.emit? About 10s or 60s I've met this issue and I found that it is because the emit is not run in the server thread of ComfyUI. That's why I added the onNextTick

tianlang0704 commented 4 months ago

Is the delay of socketio in your pc equals to the timeout option of socket.emit? About 10s or 60s I've met this issue and I found that it is because the emit is not run in the server thread of ComfyUI. That's why I added the onNextTick

Oh yeah maybe, I'll have to look deeper to find the actual cause. Right now I'm trying to use your interfaces and it seems to be happening with some randomness. Especially sends to PS triggered by message from comfyui front end was getting timeout error or was delayed for a very long time. Maybe related to the sio.call function in python, too.

PS plugin http requests seems to be affected by this, too, some how. I uncommented your timing code for send_image, and it took over 1 second to upload a 1mb image file, while the old WD version used less than 100ms.

The weirder thing is getting pixels from PS layer was also affected by it which does not involve socket message. It took about 5 seconds to get single layer data and 10 seconds to do folder merge, and a simple get_image send_image workflow in ComfyUI took about 15 seconds to run with socketio... The old WS version took less than a second...

I took a look at the PS log, it was writting super long debug log with socketio, that might be one of the causes, too.

zombieyang commented 4 months ago

There must be something wrong, I've tried your branch, and my ComfyUI became extremely slow even though I was doing nothing. the develop is ok.

Update: The problem is caused here: https://github.com/tianlang0704/sd-ppp/blob/Convert_To_Socketio/comfy/sdppp.py#L113 This function is already run in the server thread. So it is unnecessary to use _run_in_next_server_event to send the message. Even worse, _run_in_next_server_event would block the current thread (It is designed to block the Comfy Node's thread to wait for async message), which will make the comfyUI server dead.

tianlang0704 commented 4 months ago

There must be something wrong, I've tried your branch, and my ComfyUI became extremely slow even though I was doing nothing. the develop is ok.

Update: The problem is caused here: https://github.com/tianlang0704/sd-ppp/blob/Convert_To_Socketio/comfy/sdppp.py#L113 This function is already run in the server thread. So it is unnecessary to use _run_in_next_server_event to send the message. Even worse, _run_in_next_server_event would block the current thread (It is designed to block the Comfy Node's thread to wait for async message), which will make the comfyUI server dead.

Nice. I fixed the blocking problem. And I reverted back all the socket related changes. But it is still very slow, I didn't event touch the get_image code. PS would show a bar for it even.

zombieyang commented 4 months ago

There must be something wrong, I've tried your branch, and my ComfyUI became extremely slow even though I was doing nothing. the develop is ok. Update: The problem is caused here: https://github.com/tianlang0704/sd-ppp/blob/Convert_To_Socketio/comfy/sdppp.py#L113 This function is already run in the server thread. So it is unnecessary to use _run_in_next_server_event to send the message. Even worse, _run_in_next_server_event would block the current thread (It is designed to block the Comfy Node's thread to wait for async message), which will make the comfyUI server dead.

Nice. I fixed the blocking problem. And I reverted back all the socket related changes. But it is still very slow, I didn't event touch the get_image code. PS would show a bar for it even.

The code works fine in my environment now. Can your problem reproduced in a simple demo? Something like this: image

tianlang0704 commented 4 months ago

Yep. Here are 2 comparison videos using the same workflow. I ran the workflow for 3 times and then restart ComfyUI then ran the workflow for another 3 times. Socketio: 1st try: 4 seconds, 2 seconds, 2 seconds; 2nd try: 14 seconds, 4 seconds, 4 seconds https://github.com/zombieyang/sd-ppp/assets/12490479/83c8f34d-aab7-4fa9-b1a6-d5f7bd79275c Websocket: 1st try: 1 second, 1 second, 1 second; 2nd try: 1 second, 1 second, 1 second. https://github.com/zombieyang/sd-ppp/assets/12490479/1dec2c05-bbe6-4a69-aec6-e45802816b06

Edit: I just realized I accidentally folded the preview node so it's not showing anything, I don't want to re-record the video so, please ignore that.

zombieyang commented 4 months ago

I've tested main and this branch and it's actually slower. main costs 0.5s and this branch costs 3s. I will deep in later

zombieyang commented 4 months ago

some factors for slowing down:

  1. the new ServerEventLoop strategy is running in an interval of 500ms. For the case getting node, there are two calls depending on ServerEventLoop, so it would cost nearly 1000ms in the worst case. Now I change the interval to 20ms.
  2. In old code, the sending node does not await the finishing of the operation, and the latest code does. Now I remove the await.
  3. There is also an unnecessary await hostControl.resumeHistory in the frontend code and now I have removed it.

Another thing that I'm not sure, the 14s delay in your case. We know from the video that when PS shows the progress bar, the code is running this executeAsModalUntilSuccess (maybe the retry strategy is running), so can you uncomment those console.log and see the output in UXP developer tool?

tianlang0704 commented 4 months ago

I'm using the updated code here. It seem to be stuck at the upload step. The "finish upload" step actually creates a promise for uploading, and the "upload resulted" step awaits the upload. And the upload took from 1 second to 10 seconds. I think it is somehow related to the ComfyUI server thread being blocked

Log: [2024-05-10_15-03-18][31012][Default] [console] getPixels 25 ms [2024-05-10_15-03-18][31012][Default] [console] new Jimp 45 ms [2024-05-10_15-03-18][31012][Default] [console] quality 45 ms [2024-05-10_15-03-18][31012][Default] [console] create pngfile 222 ms [2024-05-10_15-03-18][31012][Default] [console] start upload 223 ms [2024-05-10_15-03-18][31012][Default] [console] finish upload 230 ms [2024-05-10_15-03-20][31012][Default] [console] upload resulted 1755 ms [2024-05-10_15-03-20][31012][Default] [console] get_image cost 1968 ms [2024-05-10_15-03-20][31012][Default] [console] imageIds.length: 1 [2024-05-10_15-03-20][31012][Default] [console] newLayerName: Comfy Images 1 [2024-05-10_15-03-21][31012][Default] [console] layer name Comfy Images 1 [2024-05-10_15-03-35][30252][Error] [console] [DP] Error fetching IMS Access token: There is no valid user profile to get an access token [2024-05-10_15-03-53][31012][Default] [console] getPixels 28 ms [2024-05-10_15-03-53][31012][Default] [console] new Jimp 47 ms [2024-05-10_15-03-53][31012][Default] [console] quality 47 ms [2024-05-10_15-03-53][31012][Default] [console] create pngfile 149 ms [2024-05-10_15-03-53][31012][Default] [console] start upload 149 ms [2024-05-10_15-03-53][31012][Default] [console] finish upload 153 ms [2024-05-10_15-03-55][31012][Default] [console] upload resulted 1275 ms [2024-05-10_15-03-55][31012][Default] [console] get_image cost 1493 ms [2024-05-10_15-03-55][31012][Default] [console] imageIds.length: 1 [2024-05-10_15-03-55][31012][Default] [console] newLayerName: Comfy Images 2 [2024-05-10_15-03-56][31012][Default] [console] layer name Comfy Images 2 [2024-05-10_15-04-05][30252][Error] [console] [DP] Error fetching IMS Access token: There is no valid user profile to get an access token [2024-05-10_15-04-35][30252][Error] [console] [DP] Error fetching IMS Access token: There is no valid user profile to get an access token [2024-05-10_15-04-36][31012][Default] [console] getPixels 33 ms [2024-05-10_15-04-36][31012][Default] [console] new Jimp 45 ms [2024-05-10_15-04-36][31012][Default] [console] quality 45 ms [2024-05-10_15-04-36][31012][Default] [console] create pngfile 170 ms [2024-05-10_15-04-36][31012][Default] [console] start upload 170 ms [2024-05-10_15-04-36][31012][Default] [console] finish upload 173 ms [2024-05-10_15-04-37][31012][Default] [console] upload resulted 1369 ms [2024-05-10_15-04-37][31012][Default] [console] get_image cost 1590 ms [2024-05-10_15-04-38][31012][Default] [console] imageIds.length: 1 [2024-05-10_15-04-38][31012][Default] [console] newLayerName: Comfy Images 3 [2024-05-10_15-04-38][31012][Default] [console] layer name Comfy Images 3 [2024-05-10_15-05-05][30252][Error] [console] [DP] Error fetching IMS Access token: There is no valid user profile to get an access token [2024-05-10_15-05-11][31012][Default] [console] disconnect [2024-05-10_15-05-11][31012][Default] [console] isConnected： false [2024-05-10_15-05-14][31012][Error] [console] connect_error reconnecting... [2024-05-10_15-05-14][31012][Default] [console] isConnected： false [2024-05-10_15-05-18][31012][Error] [console] connect_error reconnecting... [2024-05-10_15-05-18][31012][Default] [console] isConnected： false [2024-05-10_15-05-23][31012][Error] [console] connect_error reconnecting... [2024-05-10_15-05-23][31012][Default] [console] isConnected： false [2024-05-10_15-05-30][31012][Error] [console] connect_error reconnecting... [2024-05-10_15-05-30][31012][Default] [console] isConnected： false [2024-05-10_15-05-35][30252][Error] [console] [DP] Error fetching IMS Access token: There is no valid user profile to get an access token [2024-05-10_15-05-36][31012][Default] [console] connect [2024-05-10_15-05-36][31012][Default] [console] isConnected： true [2024-05-10_15-05-46][31012][Default] [console] getPixels 27 ms [2024-05-10_15-05-46][31012][Default] [console] new Jimp 30 ms [2024-05-10_15-05-46][31012][Default] [console] quality 30 ms [2024-05-10_15-05-47][31012][Default] [console] create pngfile 177 ms [2024-05-10_15-05-47][31012][Default] [console] start upload 178 ms [2024-05-10_15-05-47][31012][Default] [console] finish upload 179 ms [2024-05-10_15-05-55][31012][Default] [console] upload resulted 8873 ms [2024-05-10_15-05-56][31012][Default] [console] get_image cost 9092 ms [2024-05-10_15-05-56][31012][Default] [console] imageIds.length: 1 [2024-05-10_15-05-56][31012][Default] [console] newLayerName: Comfy Images 1 [2024-05-10_15-05-57][31012][Default] [console] layer name Comfy Images 1 [2024-05-10_15-06-02][31012][Default] [console] getPixels 48 ms [2024-05-10_15-06-02][31012][Default] [console] new Jimp 61 ms [2024-05-10_15-06-02][31012][Default] [console] quality 61 ms [2024-05-10_15-06-02][31012][Default] [console] create pngfile 232 ms [2024-05-10_15-06-02][31012][Default] [console] start upload 233 ms [2024-05-10_15-06-02][31012][Default] [console] finish upload 234 ms [2024-05-10_15-06-03][31012][Default] [console] upload resulted 1269 ms [2024-05-10_15-06-03][31012][Default] [console] get_image cost 1487 ms [2024-05-10_15-06-04][31012][Default] [console] imageIds.length: 1 [2024-05-10_15-06-04][31012][Default] [console] newLayerName: Comfy Images 2 [2024-05-10_15-06-04][31012][Default] [console] layer name Comfy Images 2 [2024-05-10_15-06-05][30252][Error] [console] [DP] Error fetching IMS Access token: There is no valid user profile to get an access token [2024-05-10_15-06-07][31012][Default] [console] getPixels 47 ms [2024-05-10_15-06-07][31012][Default] [console] new Jimp 61 ms [2024-05-10_15-06-07][31012][Default] [console] quality 61 ms [2024-05-10_15-06-08][31012][Default] [console] create pngfile 287 ms [2024-05-10_15-06-08][31012][Default] [console] start upload 288 ms [2024-05-10_15-06-08][31012][Default] [console] finish upload 291 ms [2024-05-10_15-06-09][31012][Default] [console] upload resulted 1290 ms [2024-05-10_15-06-09][31012][Default] [console] get_image cost 1507 ms [2024-05-10_15-06-09][31012][Default] [console] imageIds.length: 1 [2024-05-10_15-06-09][31012][Default] [console] newLayerName: Comfy Images 3 [2024-05-10_15-06-10][31012][Default] [console] layer name Comfy Images 3

zombieyang commented 4 months ago

There shouldn't be any block in sdppp now... the upload api is built in by ComfyUI. you can print the cost of the function here and see whether it match the cost time in frontend. https://github.com/comfyanonymous/ComfyUI/blob/master/server.py#L201

tianlang0704 commented 4 months ago

It looks like it's costing 10 seconds for fresh start, and then cost goes down to 1 - 4 seconds. I don't think it's sd-ppp, my feel is that socketio implementation in python is somewhat problematic and one have to be very careful using it in a multi-threaded environment... But that is just pure speculation, I don't have any proof. Here's the screenshots:

image

image

zombieyang commented 4 months ago

It looks like it's costing 10 seconds for fresh start, and then cost goes down to 1 - 4 seconds.

I don't think it's sd-ppp, my feel is that socketio implementation in python is somewhat problematic and one have to be very careful using it in a multi-threaded environment... But that is just pure speculation, I don't have any proof.

Here's the screenshots:

image

image

Cost in request.post....Maybe we need some performance profiles to find out more.

maybe cProfile+snakeviz could help image

zombieyang commented 4 months ago

I'll merge it first because it is a key step to develop futher feature. If performance problems still there, open another issue or PR to discuss.