gfx-rs / wgpu-rs

Rust bindings to wgpu native library
https://wgpu.rs
Mozilla Public License 2.0
1.68k stars 186 forks source link

[meta] Async all the things #175

Closed kvark closed 3 years ago

kvark commented 4 years ago

All the asynchronous methods of the Web API should be async here as well:

At the very least, we can return a future that is resolved instantly.

lachlansneff commented 4 years ago

I did some digging and as far as I can tell, SwapChain::get_next_texture cannot be made an async function (aside from returning a future that instantly resolves). I think the function that ends up getting called that may block is gfx_hal::PresentationSurface::acquire_image.

grovesNL commented 4 years ago

Some of the async API changes are available in #193, but callbacks haven't been implemented for native though.

aloucks commented 4 years ago

The call to wgpu_device_poll in the GpuFuture also seems a little odd. I believe that it can block (or soft block). I don't think Future::poll is intended to poll resources directly, but rather attempt to resolve (again) after being woken up.

https://doc.rust-lang.org/beta/std/future/trait.Future.html#runtime-characteristics

An implementation of poll should strive to return quickly, and should not block. Returning quickly prevents unnecessarily clogging up threads or event loops. If it is known ahead of time that a call to poll may end up taking awhile, the work should be offloaded to a thread pool (or something similar) to ensure that poll can return quickly.

In regards to gfx_hal::PresentationSurface::acquire_image, passing in 0 for the timeout should cause it to return immediately with NotReady if an image can't be acquired (at least in the case of vulkan). The poll result would be Pending.

What I don't understand is how it's actually getting plumbed into the runtime. With networking, the socket is registered with epoll/kqueue/iocp. It's not clear (to me) how this would work with the GPU. I think there needs to be some sort of "IO driver" that's monitoring fences/semaphores/submissions/etc and waking up tasks when the resource it's waiting on has transitioned.

lachlansneff commented 4 years ago

I agree with @aloucks. I traced device_poll all the way down and it looks like it blocks.

kvark commented 4 years ago

Polling isn't supposed to block, by definition. What makes you think it does?

aloucks commented 4 years ago

Polling isn't supposed to block, by definition. What makes you think it does?

That's what we're saying. The problem is that device_poll can block. It takes in a force_wait parameter which is set to true. I think this might be waiting for the device to become idle on every poll. Even if it were set to false it's still doing a lot of house cleaning and probably doesn't "return quickly" by the definition in the Future docs.

aloucks commented 4 years ago

I had some ideas about how to go about tackling this but it depended on timeline semaphores. It didn't look like gfx-hal supported it and I'm not sure if the other platforms have corresponding synchronization primitives.

The general idea was basically to have the Future task associated with a timeline semaphore that's submitted on the next vkQueueSubmit or vkAcquireNextImageKHR . The semaphore would be sent to a "driver" thread that's basically looping on vkWaitSemaphores with the VkSemaphoreWaitFlagBits set to VK_SEMAPHORE_WAIT_ANY_BIT. If any semaphore is triggered, wakeup all tasks and let them poll again. This all, of course, was just brainstorming. I haven't tried to flesh it all out.

kvark commented 4 years ago

Can we not discuss wgpu_device_poll at all in this issue? It's not a part of the Web API, it doesn't need to become async. In fact, it is what allows us to make the API async-ish on native (i.e. poor mans event loop).

lachlansneff commented 4 years ago

Well, device_poll is what should allow wgpu to support async. So, it seems very relevant to this issue.

grovesNL commented 4 years ago

We probably don't have to worry too much about wgpu_device_poll – we can change it however we'd like, including running on the callbacks on a separate thread if we'd prefer to do that. wgpu_device_poll is basically just an implementation detail of wgpu-native that will probably change once we decide how to integrate a native event loop. We need to continue prototyping on this and discussing the design (see https://github.com/gfx-rs/wgpu/pull/377 and https://github.com/webgpu-native/webgpu-headers/issues/18)

It would be good to change these functions to return futures (even if they're immediately resolved) as a first step towards having consistent functions signatures between native and wasm. This should be partially resolved by #204

grovesNL commented 4 years ago

I marked request adapter/device as complete here because any further changes for use of callbacks/polling/etc. for these functions would happen in wgpu-native and webgpu-headers