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

WSI: QueuePresent: canBypassXWayland(): Use latency hiding & reduce redundant requests/replies #1231

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

sharkautarch
Copy link

See: https://github.com/sharkautarch/gamescope-canBypassXWayland-jitter-tests/tree/main
(the canBypassXWayland-v2-tests folder has the measurements for this PR)
for some tests I've done measuring duration of the canBypassXWayland() calls within GamescopeWSILayer::VkDeviceOverrides::QueuePresentKHR()
the data also includes the min/max jitter (jitter being the change in the durations of consecutive canBypassXWayland() calls) for every 32 calls made

From some quick estimates off of the data from running vkcube thru gamescope (in nested mode) at 60fps, this PR reduces the median min/max (within every 32 measurements) jitter from around .60ms to ~.20ms, reduces the overall peaks in min/max jitter, and reduces the average duration from about .70ms to about .20ms

@sharkautarch sharkautarch force-pushed the QueuePresent-canBypassXWayland-v2 branch 2 times, most recently from 023e54b to d67d2af Compare April 9, 2024 15:56
@sharkautarch sharkautarch force-pushed the QueuePresent-canBypassXWayland-v2 branch from d67d2af to a3ffb8a Compare April 20, 2024 16:01
@sharkautarch
Copy link
Author

sharkautarch commented Apr 20, 2024

I've just added another edit to this PR, adding a XcbFetch template class functor to try to make the caching decision logic cleaner.
My idea behind this is:

  • Caching is now fully transparent: allowing the static xcb:: helper functions to be written as if all the work is being done serially.
  • The XcbFetch class doesn't add any runtime overhead due to the use of the constexpr constructor (the operator () method also gets inlined by the compiler)
  • Bonus feature: XcbFetch abstracts away the need to separately fetch the cookie and then the reply (while, as mentioned, allowing for transparent caching)

Let me know if the new edit is too complicated or something

@sharkautarch sharkautarch force-pushed the QueuePresent-canBypassXWayland-v2 branch 26 times, most recently from 7e3472a to 879c45a Compare April 23, 2024 21:32
@sharkautarch sharkautarch force-pushed the QueuePresent-canBypassXWayland-v2 branch 9 times, most recently from 2041344 to dc7618f Compare May 16, 2024 12:52
@sharkautarch
Copy link
Author

sharkautarch commented May 16, 2024

Last edits I made:
I simplified stuff even more.
I also switched the thread_local bool variable guarding access to g_cache to instead be a normal variable holding the owning thread's id. This should have slightly less overhead than a thread_local variable, since thread_local variables for dynamically loaded shared libraries use the general dynamic TLS model which requires running __tls_get_addr(), whereas pthread_self() just does one register move (on x86_64, and probably similar for other archs) and pthread_equal() is also pretty fast.

@sharkautarch sharkautarch force-pushed the QueuePresent-canBypassXWayland-v2 branch 3 times, most recently from 15fd60d to cee9c26 Compare May 17, 2024 14:35
@KyunLFA
Copy link

KyunLFA commented May 19, 2024

Can also confirm this PR passed a few hours of OpenGL Linux native gaming.

@sharkautarch
Copy link
Author

sharkautarch commented May 19, 2024

Can also confirm this PR passed a few hours of OpenGL Linux native gaming.

Uhhh I just realized that you said OpenGL
unless you are using Zink or something else that translates the opengl stuff to vulkan, the gamescope's WSI layer won't be loaded because gamescope WSI is a vulkan layer.

You can check if the gamescope WSI layer is running by looking for lines from the terminal output that start with [Gamescope WSI]. Where if you don't find any of the aforementioned lines, then the gamescope WSI layer didn't get loaded. (Note that you have to wait for both gamescope and the child process that it launches to start before you'll see any message about gamescope WSI)

Here's a link with info about the vulkan loader and vulkan layers: https://github.com/KhronosGroup/Vulkan-Loader/blob/main/docs/LoaderLayerInterface.md#layer-call-chains-and-distributed-dispatch

@sharkautarch sharkautarch force-pushed the QueuePresent-canBypassXWayland-v2 branch from cee9c26 to ab11589 Compare May 20, 2024 00:15
@KyunLFA
Copy link

KyunLFA commented May 20, 2024

@sharkautarch Oh wow yes, that was an embarrassing miss on my side, so sorry about that misdirect. I didn't realize at the moment I was writing my previous message that this PR is about a Vulkan layer only. Anyway, Vulkan microtests (vkcube, vkgears) are working. Will test with the latest changes as well, hopefully I can have the opportunity to test some real Vulkan games next.

@sharkautarch
Copy link
Author

@sharkautarch Oh wow yes, that was an embarrassing miss on my side, so sorry about that misdirect. I didn't realize at the moment I was writing my previous message that this PR is about a Vulkan layer only. Anyway, Vulkan microtests (vkcube, vkgears) are working. Will test with the latest changes as well, hopefully I can have the opportunity to test some real Vulkan games next.

No worries, good to hear that tests so far are going well
It's okay, we all get those minor mix ups sometimes.
Can't make an omelette without causing a few segfaults

@KyunLFA
Copy link

KyunLFA commented Jun 11, 2024

@sharkautarch

Just the FYI, it no longer works on current git HEAD. I got it working by adding

if (!gamescopeSurface->isWayland()) {
[insert_here]
}

To the line xcb::Prefetcher prefetcher(gamescopeSurface->connection, gamescopeSurface->window);

Take a look at KyunLFA@40bcd1e if interested and if you have any reservations. Found because I carry this MR in my fork :p

@sharkautarch
Copy link
Author

sharkautarch commented Jun 11, 2024

Replying to #1231 (comment)

Thanks, I’ll do that soon :)

@sharkautarch sharkautarch force-pushed the QueuePresent-canBypassXWayland-v2 branch from 6385a8c to 7b29548 Compare June 19, 2024 02:03
@sharkautarch
Copy link
Author

@KyunLFA

I updated the branch with a fix
w/ your proposed fix

if (!gamescopeSurface->isWayland()) {
xcb::Prefetcher prefetcher(gamescopeSurface->connection, gamescopeSurface->window);
}

I think that the Xcb::Prefetcher object's destructor would run after leaving the if branch, because the if branch counts as a local scope
So I instead wrote a function that returns a std::optional<Prefetcher> where it constructs the Prefetcher only when the passed boolean is true. This allows the Prefetcher object to have the right lifetime

@sharkautarch sharkautarch force-pushed the QueuePresent-canBypassXWayland-v2 branch from 7b29548 to a11a9e9 Compare June 19, 2024 02:12
@sharkautarch sharkautarch force-pushed the QueuePresent-canBypassXWayland-v2 branch from a11a9e9 to 1b59621 Compare June 19, 2024 02:21
@ninja-
Copy link

ninja- commented Sep 20, 2024

👀

ProjectSynchro added a commit to ProjectSynchro/copr-gamescope-git that referenced this pull request Oct 5, 2024
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.

3 participants