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

RSX: wait when emulation is paused (reduces CPU usage) #10837

Merged
merged 2 commits into from Sep 17, 2021

Conversation

Megamouse
Copy link
Contributor

This decreases my cpu usage in GMPADTEST from ~9% to <1% during Emu.Pause() (together with the other pad_thread PR)

@Megamouse Megamouse added RSX Optimization Optimizes existing code labels Sep 7, 2021
@Megamouse Megamouse requested a review from kd-11 September 7, 2021 21:41
@Megamouse
Copy link
Contributor Author

I'm sure there's a better way.
But at least this raises awareness. WAKE UP

@elad335
Copy link
Contributor

elad335 commented Sep 8, 2021

Should be at rsx::thread::cpu_wait() while using the state value argument. (currently unused)

@kd-11
Copy link
Contributor

kd-11 commented Sep 8, 2021

Relates to #9641, specifically pausing this way will make that ticket unactionable. Maybe add fixes link for that to this PR then.

@Megamouse
Copy link
Contributor Author

Maybe we can simply add a 16ms wait to the main loop instead.
I'll test the impact on CPU usage.

Maybe it's time to implement that pause screen 🤣

@kd-11
Copy link
Contributor

kd-11 commented Sep 8, 2021

You can do it the same as shader loading dialog, just create a custom interface with low opacity that is updated every 16ms and calls flip in the update method, then trigger update from inside the paused loop. Just remember to destroy it once the pause ends.

@Megamouse
Copy link
Contributor Author

We can still pause rsx regardless of the issue ticket and the pause dialog though, until someone implements either.

I just don't see a reason to destroy the planet any more than we have to for no reason.

@kd-11
Copy link
Contributor

kd-11 commented Sep 8, 2021

That was my original argument in that other ticket, you either waste resources drawing a pause screen or you leave it empty. Since the choice is to conserve resources and you are the reporter of the other ticket, I really see no point in handling it. Or do you still want a pause screen? Remember it will literally undo some of the power savings here.

@Megamouse
Copy link
Contributor Author

There are a couple of things to consider here:
For example once we have pause screen we can still pause RSX while the window is minimized.
And I still think it's weird that we can't access the last frame. Either through Qt shenanigans or by accessing some kind of framebuffer

@elad335
Copy link
Contributor

elad335 commented Sep 8, 2021

Accessing last frame would be nice for savestates "start paused" loading mode as well, currently it just waits till one more frame is rendered then pause.

@kd-11
Copy link
Contributor

kd-11 commented Sep 8, 2021

The problem is how the OS handles painting the window - if you resize/minimize/move etc it is up to the application to handle the event, otherwise the window is just going to have garbage in it. Of course for this to happen would mean RSX thread has to be awake to repaint everything and this causes another hilarious problem - you cannot tell that the window size changed without polling unless you set up your own notification hooks.
Combine these 2 requirements and suddenly you cannot really keep RSX asleep, you have to keep waking it periodically to poll and handle events.

@kd-11
Copy link
Contributor

kd-11 commented Sep 8, 2021

I'm not saying its not doable or anything, just that it will eat up some system resources for visual consistency. I can set up the basic framework for it if it is decided to go that route, but RIP power savings is going to be the problem.

@Megamouse
Copy link
Contributor Author

But if it's event based then you can just wake up the thread for a blip and do the necessary work in one iteration can't you?

@kd-11
Copy link
Contributor

kd-11 commented Sep 8, 2021

But if it's event based then you can just wake up the thread for a blip and do the necessary work in one iteration can't you?

It is not event based, I explained that above. It is event-based from the OS level (win32, X11) but not on our side (Qt, Vulkan)
e.g The easiest way to tell that you can safely resize your vulkan window is to try and draw something on it and the driver will return a code that tells you you need to refresh the swapchain. Or you can poll window size changes - again this depends on the driver. The way I see it, we will need some kind of periodic refresh going to trigger this, which will mean sending things to be rendered (you can just call flip(0) for the last flipped image)

@MSuih
Copy link
Member

MSuih commented Sep 8, 2021

Can't we catch the event with Qt?

@kd-11
Copy link
Contributor

kd-11 commented Sep 8, 2021

Can't we catch the event with Qt?

I remember this does not work. Previously there was some Msq injection I had added to gsframe that allowed me to snoop on the win32 activity before the driver could start acting on it, but it ended up being really dangerous and it was removed. it has to do with timing, if you catch a QT event, wake up and do something, you must be sure the driver does not trash your decision immediately after. Things my have improved since then, though I haven't really checked.

@kd-11
Copy link
Contributor

kd-11 commented Sep 8, 2021

Since there is still a lot of discussion, let's keep the other ticket open then. I'll have to check later the best place to add the sleep.

@Megamouse Megamouse marked this pull request as draft September 9, 2021 17:32
Copy link
Contributor

@kd-11 kd-11 left a comment

Choose a reason for hiding this comment

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

Ramblings:

  1. rsx::thread now derives from cpu_thread which has some simpler ways of checking if paused without being stopped. You can just check if state & (cpu_flag::dbg_global_pause + cpu_flag::exit) == cpu_flag::dbg_global_pause to check if only pause is enabled. However, this isn't even necessary due to point 2...
  2. As elad mentioned, we already have some logic to optionally pause things in cpu_wait() wich is called every iteration whenever test_stopped() is called. You should add your sleep in there based on the state value. You should even just sleep once (16ms) and let the system continue which allows RSX to keep operating in a low power mode and is able to react to other requests such as screen refreshes and such.

@Megamouse
Copy link
Contributor Author

Updated PR with the suggested solution (hopefully correct).
Tested with pausing and also stopping while being paused.

@Megamouse Megamouse marked this pull request as ready for review September 16, 2021 20:18
@AniLeo AniLeo requested a review from kd-11 September 16, 2021 20:49
rpcs3/Emu/RSX/RSXThread.cpp Outdated Show resolved Hide resolved
This decreases my cpu usage by to <1% during Emu.Pause()
@Megamouse Megamouse changed the title rsx: wait when emulation is paused RSX: wait when emulation is paused (reduces CPU usage) Sep 17, 2021
@Megamouse Megamouse merged commit 14a425e into RPCS3:master Sep 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Optimization Optimizes existing code RSX
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants