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

Remove sleep in upscale #2853

Merged
merged 3 commits into from
May 17, 2024
Merged

Remove sleep in upscale #2853

merged 3 commits into from
May 17, 2024

Conversation

joeyballentine
Copy link
Member

I'm assuming this has a reason to be here, otherwise I can't see why it's worth slightly slowing down every upscale.

If it doesn't, let's get rid of it. If it does, can we use a smaller number? Even though it's only a millisecond, that time can add up. A 5 minute video clip at 24fps would cost about 7 extra seconds in processing time because of this.

@RunDevelopment
Copy link
Member

I'm assuming this has a reason to be here

Because Python is a single-threaded piece of shit that I don't trust at all. Basically, I don't think Python does time division multi-threading, which means that one running thread with starve all other threads. Normally, blocking IO operations make threads yield, so other threads can get some CPU, but we are only doing computations. So we need some way for threads to yield and let other threads and let other threads do some works. In particular, the thread that accepts the /pause request would get starved if we don't sleep here, which makes pausing impossible.

That said, time.sleep(0) is supposed to be make a thread yield without actually sleeping any time, but I think that didn't work when I tested it. I could be misremembering, though.

So you definitely can't remove the sleep, but you could try replacing it with time.sleep(0) and testing whether pausing still works.

can we use a smaller number

Yes, but it probably won't do much. Windows' native sleep kernel API only supports millisecond precision. There is a high-precision sleep API, but I don't know if Python uses that. On top of that, Windows likes to sleep a little longer sometimes...

As I said above, you can try 0, though. Python has a special cause for time.sleep(0), where it doesn't actually use the OS' sleep.

@joeyballentine
Copy link
Member Author

Ok, I'll try

@joeyballentine
Copy link
Member Author

Works perfectly fine. Tested by using a heavy model with 128 tile size. Was able to pause a few times while it was processing. I could actually hear my GPU stop processing while paused. Resuming eventually led to it finishing.

RunDevelopment
RunDevelopment previously approved these changes May 17, 2024
Copy link
Member

@RunDevelopment RunDevelopment left a comment

Choose a reason for hiding this comment

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

Let's add a comment explaining the thread.sleep(0).

After that, feel free to merge this.

Co-authored-by: Michael Schmidt <[email protected]>
@joeyballentine joeyballentine merged commit 94db450 into main May 17, 2024
14 checks passed
@joeyballentine joeyballentine deleted the remove-sleep branch May 17, 2024 16:21
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.

2 participants