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

Non-blocking API #1451

Closed
LaylBongers opened this issue Dec 13, 2019 · 16 comments
Closed

Non-blocking API #1451

LaylBongers opened this issue Dec 13, 2019 · 16 comments
Labels
area: api Issues related to API surface area: ecosystem Help the connected projects grow and prosper type: enhancement New feature or request

Comments

@LaylBongers
Copy link
Contributor

I'm honestly not sure entirely where I should post this, given the different projects that cover what goes into wgpu, but I figured I would put it here, even if perhaps it needs to be delegated upstream.

I've been working with projects that require rendering across multiple windows, as well as trying to fit the entire project on an async runtime (tokio in my case) to parallelize my projects better, and make better use of worker threads.

At this time WGPU has a function that prevents me from rendering to multiple windows efficiently without creating a thread per window, or putting WGPU rendering code on the tokio runtime. Specifically the function SwapChain::get_next_texture.

On native platforms, this function will block until the next texture is available. This means that, without using 1 thread per window, all rendering and updating of all windows will stall whenever an attempt is made to render to one window, without any way to in advance detect if it will block.

This also means it can't be used on a tokio runtime, as multiple windows being rendered to will effectively stall worker threads until it's done blocking.

It would be nice if there was an alternative function, or a feature on this function, to never block but instead poll. At the very least the Vulkan backend supports this behavior on acquiring the next swapchain image. This would also make it possible to implement a Future around the SwapChain and use WGPU on a tokio runtime without blocking the worker thread.

Currently, I don't know if there's any other functions in WGPU that have similar issues, but if there are it would be nice to, where possible, have similar non-blocking APIs available.

@kvark
Copy link
Member

kvark commented Dec 16, 2019

I think having a Future-based get_next_texture would be great! Not sure if it makes sense to remove the current one though: most clients are not async, and Future would just be a headache to deal with.

@LaylBongers
Copy link
Contributor Author

LaylBongers commented Dec 16, 2019

I can add a variation of get_next_texture to wgpu-native that polls rather than blocks, and add the bindings for that to wgpu-rs. I can then add a Future version that uses that function. Does this require any change to upstream headers or can I just add it?

@kvark
Copy link
Member

kvark commented Dec 16, 2019

We are going to need to discuss this with @Kangz. Do you have thoughts on poll-based (or asynchronous) way of getting the next texture from the swapchain?

@LaylBongers
Copy link
Contributor Author

LaylBongers commented Dec 16, 2019

In vulkan, vkAcquireNextImageKHR can be given a timeout of 0 to poll for available images:

If timeout is 0, vkAcquireNextImageKHR will not block, but will either succeed or return VK_NOT_READY.

My assumption was that the timeout in gfx-hal functions similar, but I may be wrong on that.

My plan is to use this to create a function in WGPU that will simply poll for an image, and return the image to the caller if one has been acquired, or return that none could be acquired yet.

Since Future in Rust works by polling if the result is available, this is all that's necessary to create an async implementation.

@CryZe
Copy link

CryZe commented Dec 16, 2019

Since Future in Rust works by polling if the result is available, this is all that's necessary to create an async implementation.

That is not true. You need a driver / reactor (There doesn't seem to be a conclusive name. Tokio calls it a driver atm) that calls the waker when the future is ready.

@Kangz
Copy link

Kangz commented Dec 16, 2019

Thanks @kvark for the head's up. @LaylConway as far as I can see, no other synchronous WebGPU function can block.

I wasn't able to find spec language for Vulkan that guarantees that vkAcquireNextImage will return immediately. The closest section is the following, but it doesn't help very much:

Let n be the total number of images in the swapchain, m be the value of VkSurfaceCapabilitiesKHR::minImageCount, and a be the number of presentable images that the application has currently acquired (i.e. images acquired with vkAcquireNextImageKHR, but not yet presented with vkQueuePresentKHR). vkAcquireNextImageKHR can always succeed if a ≤ n - m at the time vkAcquireNextImageKHR is called. vkAcquireNextImageKHR should not be called if a > n - m with a timeout of UINT64_MAX; in such a case, vkAcquireNextImageKHR may block indefinitely.

WebGPU's GPUSwapChain.getCurrentTexture can't block so any async wgpuSwapChainGetCurrentTextureView would be trivially implementable there. I don't have a strong opinion on the shape of the API for that. A callback seems a bit heavy-handed, so maybe it could be a flag on the WGPUSwapChainDescriptor that says wgpuSwapChainGetCurrentTextureView can return null if nothing is ready? Either way would work for Dawn.

@kvark
Copy link
Member

kvark commented Dec 16, 2019

Ok, great, thanks for feedback!
@LaylConway let's proceed with the poll

@CryZe
Copy link

CryZe commented Dec 16, 2019

You either want to pass some callback or specify some small timeout for the future. Otherwise that will either deadlock or be a really hot spinloop. (Or really just use tokio's spawn_blocking API, which is meant for use cases like this)

@LaylBongers
Copy link
Contributor Author

I think I'll have to do a bit more research into Futures then, I admit my knowledge on asnyc is very incomplete. Myself I just need a non-blocking call, and being able to wrap a Future around it would just be a nice-to-have. I'll ask for some help on getting a more complete picture of Futures when I've got the time later today.

The main reason I didn't want to go with spawn_blocking is that it seems to, to me, that it would keep a lot of threads blocking and quickly exhaust the thread pool if a bunch of windows of an application are open.

Once I can get a few outstanding questions of this implementation idea answered, I'll see if I can put together a PR.

@LaylBongers
Copy link
Contributor Author

I made an attempt at implementing this. However it seems that on my (recently updated) NVIDIA driver, acquiring an image never blocks. Instead, presenting blocks.
I've narrowed it down to that calling queue.present_surface in gfx-hal is a blocking operation when VSync is enabled.
I've searched around and it seems that, at least on NVIDIA, others are reporting vkQueuePresentKHR is a blocking operation if VSync is enabled. To me this seems like unexpected behavior, since presenting has no timeout.

I'm not sure how to move forward from here. To me it seems like this isn't how the API is designed to work, and NVIDIA has a problem with their driver implementation.

@kvark
Copy link
Member

kvark commented Dec 20, 2019

This is surprising and worthy of following up with Nvidia. But does it matter to the API here? Presenting is a one-way command, whether it blocks or not doesn't really matter to the caller, unlike acquiring the image, which has to return something.

@LaylBongers
Copy link
Contributor Author

It keeps a thread busy though. I'm rendering to multiple windows, so this is something I'll have to keep in mind as it could keep the rendering thread busy just waiting instead of drawing other windows, slowing down the rest.
It also keeps a thread busy for at worst ~16ms which could affect the performance of anything else on the thread pool if run on the regular tokio thread pool, so I'll have to design around it. Because of SwapChainOutput borrowing as well, I can't easily send it to another thread.

@kvark
Copy link
Member

kvark commented Dec 20, 2019

I see. We only have NVidia + Vulkan to blame for that. Want to raise an issue on, say, https://github.com/KhronosGroup/Vulkan-Docs ? That would be great, I'll be happy to assist in discussions, where possible.

@LaylBongers
Copy link
Contributor Author

I may later raise an issue there, not sure if this is a docs issue though.
I'm meanwhile trying to figure out a workaround.

There's some other issues that make trying to acquire an image non-blocking hard.
It seems sometimes when you wait with a timeout of 0, gfx-hal will panic, rather than return Timeout or NotReady.
As well, not all backends respect the timeout value it seems. I can also imagine more backends block on present rather than image acquiring.

I'm trying out a different approach in my code right now which does just shift the blocking tasks to spawn_blocking in tokio, but this is hard to do because of SwapChainOutput requiring a borrow of the SwapChain. block_in_place works but seems to be a more expensive operation, as it turns the worker thread into a blocking thread and shifts all previously assigned work to other threads.

@kvark
Copy link
Member

kvark commented Dec 20, 2019

Interesting. I do admit the new swapchain model is guilty of sometimes ignoring the timeouts. We need to fix that (having issues filed, etc), as well as the panic thing you are describing.

@kvark kvark transferred this issue from gfx-rs/wgpu-rs Jun 3, 2021
@kvark kvark added area: api Issues related to API surface area: ecosystem Help the connected projects grow and prosper type: enhancement New feature or request labels Jun 3, 2021
@cwfitzgerald
Copy link
Member

I'm going to close this as stale as so much has changed, there are still similar issues open - the current idea is to move get_current_texture onto another thread. There will also be ways to control frame timing such that get_current_texture never waits. See #2650 for another tool that will help you achieve this.

@cwfitzgerald cwfitzgerald closed this as not planned Won't fix, can't repro, duplicate, stale Jun 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: api Issues related to API surface area: ecosystem Help the connected projects grow and prosper type: enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

5 participants