Skip to content
This repository was archived by the owner on Jun 18, 2021. It is now read-only.

[meta] Async all the things #175

Closed
3 of 4 tasks
kvark opened this issue Feb 10, 2020 · 11 comments
Closed
3 of 4 tasks

[meta] Async all the things #175

kvark opened this issue Feb 10, 2020 · 11 comments
Labels
enhancement New feature or request help wanted Extra attention is needed

Comments

@kvark
Copy link
Member

kvark commented Feb 10, 2020

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

  • buffer mapping
  • request adapter
  • request device
  • get next swapchain texture

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

@kvark kvark added enhancement New feature or request help wanted Extra attention is needed labels Feb 10, 2020
@kvark kvark added this to the Version 0.5 milestone Feb 10, 2020
@lachlansneff
Copy link
Contributor

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
Copy link
Collaborator

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

@aloucks
Copy link
Contributor

aloucks commented Mar 16, 2020

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
Copy link
Contributor

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

@kvark
Copy link
Member Author

kvark commented Mar 16, 2020

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

@aloucks
Copy link
Contributor

aloucks commented Mar 16, 2020

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
Copy link
Contributor

aloucks commented Mar 16, 2020

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
Copy link
Member Author

kvark commented Mar 16, 2020

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
Copy link
Contributor

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

@grovesNL
Copy link
Collaborator

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 gfx-rs/wgpu#377 and webgpu-native/webgpu-headers#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

bors bot added a commit that referenced this issue Mar 16, 2020
204: Return futures from request adapter/request device r=kvark a=grovesNL

Relevant to #175

Backporting some parts of #193 for async request adapter and async request device.

Co-authored-by: Joshua Groves <[email protected]>
bors bot added a commit that referenced this issue Mar 16, 2020
204: Return futures from request adapter/request device r=kvark a=grovesNL

Relevant to #175

Backporting some parts of #193 for async request adapter and async request device.

Co-authored-by: Joshua Groves <[email protected]>
@grovesNL
Copy link
Collaborator

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

@kvark kvark modified the milestones: Version 0.5, Version 0.6 Jul 23, 2020
@kvark kvark removed this from the Version 0.6 milestone Aug 18, 2020
@kvark kvark closed this as completed Mar 2, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

4 participants