Merge #1600

1600: Vulkan Timeline Semaphores r=kvark a=kvark

**Connections**
None

**Description**
One of the non-straightforward design decisions in wgpu-hal was to expose the fences with `D3D12Fence` semantics. Now that we got Vulkan, Metal, and GLES3, it became a bit concerning that all of these backends implemented this new fence API in the same convoluted way: via managing a pool of binary fences internally, which requires maintenance (done on submit() only). If that's the expected implementation, then the abstraction is wrong.

For this reason, it was important for me to try implementing this new semantics in a modern Vulkan way - via timeline semaphores. With this, we have 1:1 relation between `Api::Fence` and a Vulkan object. I believe the experiment is a success: Vulkan is now very efficient at fences!

**Testing**
Works on the examples!


Co-authored-by: Dzmitry Malyshau <dmalyshau@mozilla.com>

wgpu-hal/src/vulkan/adapter.rs

@@ -21,6 +21,7 @@ pub struct PhysicalDeviceFeatures {
     vulkan_1_2: Option<vk::PhysicalDeviceVulkan12Features>,
     descriptor_indexing: Option<vk::PhysicalDeviceDescriptorIndexingFeaturesEXT>,
     imageless_framebuffer: Option<vk::PhysicalDeviceImagelessFramebufferFeaturesKHR>,
+    timeline_semaphore: Option<vk::PhysicalDeviceTimelineSemaphoreFeaturesKHR>,
 }
 
 // This is safe because the structs have `p_next: *mut c_void`, which we null out/never read.
@@ -165,6 +166,7 @@ impl PhysicalDeviceFeatures {
                         )
                         //.sampler_filter_minmax(requested_features.contains(wgt::Features::SAMPLER_REDUCTION))
                         .imageless_framebuffer(private_caps.imageless_framebuffers)
+                        .timeline_semaphore(private_caps.timeline_semaphores)
                         .build(),
                 )
             } else {
@@ -221,6 +223,16 @@ impl PhysicalDeviceFeatures {
             } else {
                 None
             },
+            timeline_semaphore: if enabled_extensions.contains(&vk::KhrTimelineSemaphoreFn::name())
+            {
+                Some(
+                    vk::PhysicalDeviceTimelineSemaphoreFeaturesKHR::builder()
+                        .timeline_semaphore(true)
+                        .build(),
+                )
+            } else {
+                None
+            },
         }
     }
 
@@ -539,6 +551,15 @@ impl super::InstanceShared {
                 mut_ref.p_next = mem::replace(&mut features2.p_next, mut_ref as *mut _ as *mut _);
             }
 
+            // `VK_KHR_timeline_semaphore` is promoted to 1.2, but has no changes, so we can keep using the extension unconditionally.
+            if capabilities.supports_extension(vk::KhrTimelineSemaphoreFn::name()) {
+                features.timeline_semaphore =
+                    Some(vk::PhysicalDeviceTimelineSemaphoreFeaturesKHR::builder().build());
+
+                let mut_ref = features.timeline_semaphore.as_mut().unwrap();
+                mut_ref.p_next = mem::replace(&mut features2.p_next, mut_ref as *mut _ as *mut _);
+            }
+
             unsafe {
                 get_device_properties.get_physical_device_features2_khr(phd, &mut features2);
             }
@@ -560,6 +581,7 @@ impl super::InstanceShared {
             null_p_next(&mut features.vulkan_1_2);
             null_p_next(&mut features.descriptor_indexing);
             null_p_next(&mut features.imageless_framebuffer);
+            null_p_next(&mut features.timeline_semaphore);
         }
 
         (capabilities, features)
@@ -637,12 +659,16 @@ impl super::Instance {
         let private_caps = super::PrivateCapabilities {
             flip_y_requires_shift: phd_capabilities.properties.api_version >= vk::API_VERSION_1_1
                 || phd_capabilities.supports_extension(vk::KhrMaintenance1Fn::name()),
-            imageless_framebuffers: phd_features
-                .vulkan_1_2
-                .map_or(false, |features| features.imageless_framebuffer == vk::TRUE)
-                || phd_capabilities.supports_extension(vk::KhrImagelessFramebufferFn::name()),
+            imageless_framebuffers: match phd_features.vulkan_1_2 {
+                Some(features) => features.imageless_framebuffer == vk::TRUE,
+                None => phd_capabilities.supports_extension(vk::KhrImagelessFramebufferFn::name()),
+            },
             image_view_usage: phd_capabilities.properties.api_version >= vk::API_VERSION_1_1
                 || phd_capabilities.supports_extension(vk::KhrMaintenance2Fn::name()),
+            timeline_semaphores: match phd_features.vulkan_1_2 {
+                Some(features) => features.timeline_semaphore == vk::TRUE,
+                None => phd_capabilities.supports_extension(vk::KhrTimelineSemaphoreFn::name()),
+            },
             texture_d24: unsafe {
                 self.shared
                     .raw
@@ -775,6 +801,17 @@ impl crate::Adapter<super::Api> for super::Adapter {
         } else {
             None
         };
+        let timeline_semaphore_fn = if enabled_extensions.contains(&khr::TimelineSemaphore::name())
+        {
+            Some(super::ExtensionFn::Extension(khr::TimelineSemaphore::new(
+                &self.instance.entry,
+                &self.instance.raw,
+            )))
+        } else if self.phd_capabilities.properties.api_version >= vk::API_VERSION_1_2 {
+            Some(super::ExtensionFn::Promoted)
+        } else {
+            None
+        };
 
         let naga_options = {
             use naga::back::spv;
@@ -808,6 +845,7 @@ impl crate::Adapter<super::Api> for super::Adapter {
             instance: Arc::clone(&self.instance),
             extension_fns: super::DeviceExtensionFunctions {
                 draw_indirect_count: indirect_count_fn,
+                timeline_semaphore: timeline_semaphore_fn,
             },
             vendor_id: self.phd_capabilities.properties.vendor_id,
             downlevel_flags: self.downlevel_flags,

wgpu-hal/src/vulkan/device.rs

@@ -1,7 +1,11 @@
 use super::conv;
 
 use arrayvec::ArrayVec;
-use ash::{extensions::khr, version::DeviceV1_0, vk};
+use ash::{
+    extensions::khr,
+    version::{DeviceV1_0, DeviceV1_2},
+    vk,
+};
 use inplace_it::inplace_or_alloc_from_iter;
 use parking_lot::Mutex;
 
@@ -1390,26 +1394,49 @@ impl crate::Device<super::Api> for super::Device {
     unsafe fn destroy_query_set(&self, set: super::QuerySet) {
         self.shared.raw.destroy_query_pool(set.raw, None);
     }
+
     unsafe fn create_fence(&self) -> Result<super::Fence, crate::DeviceError> {
-        Ok(super::Fence {
-            last_completed: 0,
-            active: Vec::new(),
-            free: Vec::new(),
+        Ok(if self.shared.private_caps.timeline_semaphores {
+            let mut sem_type_info =
+                vk::SemaphoreTypeCreateInfo::builder().semaphore_type(vk::SemaphoreType::TIMELINE);
+            let vk_info = vk::SemaphoreCreateInfo::builder().push_next(&mut sem_type_info);
+            let raw = self.shared.raw.create_semaphore(&vk_info, None)?;
+            super::Fence::TimelineSemaphore(raw)
+        } else {
+            super::Fence::FencePool {
+                last_completed: 0,
+                active: Vec::new(),
+                free: Vec::new(),
+            }
         })
     }
     unsafe fn destroy_fence(&self, fence: super::Fence) {
-        for (_, raw) in fence.active {
-            self.shared.raw.destroy_fence(raw, None);
-        }
-        for raw in fence.free {
-            self.shared.raw.destroy_fence(raw, None);
+        match fence {
+            super::Fence::TimelineSemaphore(raw) => {
+                self.shared.raw.destroy_semaphore(raw, None);
+            }
+            super::Fence::FencePool {
+                active,
+                free,
+                last_completed: _,
+            } => {
+                for (_, raw) in active {
+                    self.shared.raw.destroy_fence(raw, None);
+                }
+                for raw in free {
+                    self.shared.raw.destroy_fence(raw, None);
+                }
+            }
         }
     }
     unsafe fn get_fence_value(
         &self,
         fence: &super::Fence,
     ) -> Result<crate::FenceValue, crate::DeviceError> {
-        fence.get_latest(&self.shared.raw)
+        fence.get_latest(
+            &self.shared.raw,
+            self.shared.extension_fns.timeline_semaphore.as_ref(),
+        )
     }
     unsafe fn wait(
         &self,
@@ -1417,21 +1444,50 @@ impl crate::Device<super::Api> for super::Device {
         wait_value: crate::FenceValue,
         timeout_ms: u32,
     ) -> Result<bool, crate::DeviceError> {
-        if wait_value <= fence.last_completed {
-            Ok(true)
-        } else {
-            match fence.active.iter().find(|&&(value, _)| value >= wait_value) {
-                Some(&(_, raw)) => {
-                    let timeout_us = timeout_ms as u64 * super::MILLIS_TO_NANOS;
-                    match self.shared.raw.wait_for_fences(&[raw], true, timeout_us) {
-                        Ok(()) => Ok(true),
-                        Err(vk::Result::TIMEOUT) => Ok(false),
-                        Err(other) => Err(other.into()),
+        let timeout_us = timeout_ms as u64 * super::MILLIS_TO_NANOS;
+        match *fence {
+            super::Fence::TimelineSemaphore(raw) => {
+                let semaphores = [raw];
+                let values = [wait_value];
+                let vk_info = vk::SemaphoreWaitInfo::builder()
+                    .semaphores(&semaphores)
+                    .values(&values);
+                let result = match self.shared.extension_fns.timeline_semaphore {
+                    Some(super::ExtensionFn::Extension(ref ext)) => {
+                        ext.wait_semaphores(self.shared.raw.handle(), &vk_info, timeout_us)
                     }
+                    Some(super::ExtensionFn::Promoted) => {
+                        self.shared.raw.wait_semaphores(&vk_info, timeout_us)
+                    }
+                    None => unreachable!(),
+                };
+                match result {
+                    Ok(()) => Ok(true),
+                    Err(vk::Result::TIMEOUT) => Ok(false),
+                    Err(other) => Err(other.into()),
                 }
-                None => {
-                    log::error!("No signals reached value {}", wait_value);
-                    Err(crate::DeviceError::Lost)
+            }
+            super::Fence::FencePool {
+                last_completed,
+                ref active,
+                free: _,
+            } => {
+                if wait_value <= last_completed {
+                    Ok(true)
+                } else {
+                    match active.iter().find(|&&(value, _)| value >= wait_value) {
+                        Some(&(_, raw)) => {
+                            match self.shared.raw.wait_for_fences(&[raw], true, timeout_us) {
+                                Ok(()) => Ok(true),
+                                Err(vk::Result::TIMEOUT) => Ok(false),
+                                Err(other) => Err(other.into()),
+                            }
+                        }
+                        None => {
+                            log::error!("No signals reached value {}", wait_value);
+                            Err(crate::DeviceError::Lost)
+                        }
+                    }
                 }
             }
         }

wgpu-hal/src/vulkan/instance.rs

@@ -125,7 +125,7 @@ impl super::Instance {
         }
 
         let surface = {
-            let xlib_loader = khr::XlibSurface::new(&self.entry, &self.shared.raw);
+            let xlib_loader = khr::XlibSurface::new(&self.shared.entry, &self.shared.raw);
             let info = vk::XlibSurfaceCreateInfoKHR::builder()
                 .flags(vk::XlibSurfaceCreateFlagsKHR::empty())
                 .window(window)
@@ -149,7 +149,7 @@ impl super::Instance {
         }
 
         let surface = {
-            let xcb_loader = khr::XcbSurface::new(&self.entry, &self.shared.raw);
+            let xcb_loader = khr::XcbSurface::new(&self.shared.entry, &self.shared.raw);
             let info = vk::XcbSurfaceCreateInfoKHR::builder()
                 .flags(vk::XcbSurfaceCreateFlagsKHR::empty())
                 .window(window)
@@ -173,7 +173,7 @@ impl super::Instance {
         }
 
         let surface = {
-            let w_loader = khr::WaylandSurface::new(&self.entry, &self.shared.raw);
+            let w_loader = khr::WaylandSurface::new(&self.shared.entry, &self.shared.raw);
             let info = vk::WaylandSurfaceCreateInfoKHR::builder()
                 .flags(vk::WaylandSurfaceCreateFlagsKHR::empty())
                 .display(display)
@@ -188,7 +188,7 @@ impl super::Instance {
     #[allow(dead_code)]
     fn create_surface_android(&self, window: *const c_void) -> super::Surface {
         let surface = {
-            let a_loader = khr::AndroidSurface::new(&self.entry, &self.shared.raw);
+            let a_loader = khr::AndroidSurface::new(&self.shared.entry, &self.shared.raw);
             let info = vk::AndroidSurfaceCreateInfoKHR::builder()
                 .flags(vk::AndroidSurfaceCreateFlagsKHR::empty())
                 .window(window as *mut _);
@@ -214,7 +214,7 @@ impl super::Instance {
                 .flags(vk::Win32SurfaceCreateFlagsKHR::empty())
                 .hinstance(hinstance)
                 .hwnd(hwnd);
-            let win32_loader = khr::Win32Surface::new(&self.entry, &self.shared.raw);
+            let win32_loader = khr::Win32Surface::new(&self.shared.entry, &self.shared.raw);
             unsafe {
                 win32_loader
                     .create_win32_surface(&info, None)
@@ -277,7 +277,7 @@ impl super::Instance {
     }
 
     fn create_surface_from_vk_surface_khr(&self, surface: vk::SurfaceKHR) -> super::Surface {
-        let functor = khr::Surface::new(&self.entry, &self.shared.raw);
+        let functor = khr::Surface::new(&self.shared.entry, &self.shared.raw);
         super::Surface {
             raw: surface,
             functor,
@@ -287,21 +287,14 @@ impl super::Instance {
     }
 }
 
-impl Drop for super::Instance {
+impl Drop for super::InstanceShared {
     fn drop(&mut self) {
-        match Arc::get_mut(&mut self.shared) {
-            Some(shared) => unsafe {
-                if let Some(du) = shared.debug_utils.take() {
-                    du.extension
-                        .destroy_debug_utils_messenger(du.messenger, None);
-                }
-            },
-            None => {
-                log::error!("Unable to drop Instance: something created from it is still alive");
-            }
-        }
         unsafe {
-            self.shared.raw.destroy_instance(None);
+            if let Some(du) = self.debug_utils.take() {
+                du.extension
+                    .destroy_debug_utils_messenger(du.messenger, None);
+            }
+            self.raw.destroy_instance(None);
         }
     }
 }
@@ -495,9 +488,9 @@ impl crate::Instance<super::Api> for super::Instance {
                 flags: desc.flags,
                 debug_utils,
                 get_physical_device_properties,
+                entry,
             }),
             extensions,
-            entry,
         })
     }
 

wgpu-hal/src/vulkan/mod.rs

@@ -18,6 +18,11 @@ any of the image views (they have) gets removed.
 If Vulkan supports image-less framebuffers,
 then the actual views are excluded from the framebuffer key.
 
+## Fences
+
+If timeline semaphores are available, they are used 1:1 with wgpu-hal fences.
+Otherwise, we manage a pool of `VkFence` objects behind each `hal::Fence`.
+
 !*/
 
 mod adapter;
@@ -31,7 +36,7 @@ use std::{borrow::Borrow, ffi::CStr, sync::Arc};
 use arrayvec::ArrayVec;
 use ash::{
     extensions::{ext, khr},
-    version::DeviceV1_0,
+    version::{DeviceV1_0, DeviceV1_2},
     vk,
 };
 use parking_lot::Mutex;
@@ -78,12 +83,12 @@ struct InstanceShared {
     flags: crate::InstanceFlags,
     debug_utils: Option<DebugUtils>,
     get_physical_device_properties: Option<vk::KhrGetPhysicalDeviceProperties2Fn>,
+    entry: ash::Entry,
 }
 
 pub struct Instance {
     shared: Arc<InstanceShared>,
     extensions: Vec<&'static CStr>,
-    entry: ash::Entry,
 }
 
 struct Swapchain {
@@ -135,6 +140,7 @@ enum ExtensionFn<T> {
 
 struct DeviceExtensionFunctions {
     draw_indirect_count: Option<ExtensionFn<khr::DrawIndirectCount>>,
+    timeline_semaphore: Option<ExtensionFn<khr::TimelineSemaphore>>,
 }
 
 /// Set of internal capabilities, which don't show up in the exposed
@@ -147,6 +153,7 @@ struct PrivateCapabilities {
     flip_y_requires_shift: bool,
     imageless_framebuffers: bool,
     image_view_usage: bool,
+    timeline_semaphores: bool,
     texture_d24: bool,
     texture_d24_s8: bool,
     non_coherent_map_mask: wgt::BufferAddress,
@@ -349,17 +356,23 @@ pub struct QuerySet {
 }
 
 #[derive(Debug)]
-pub struct Fence {
-    last_completed: crate::FenceValue,
-    /// The pending fence values have to be ascending.
-    active: Vec<(crate::FenceValue, vk::Fence)>,
-    free: Vec<vk::Fence>,
+pub enum Fence {
+    TimelineSemaphore(vk::Semaphore),
+    FencePool {
+        last_completed: crate::FenceValue,
+        /// The pending fence values have to be ascending.
+        active: Vec<(crate::FenceValue, vk::Fence)>,
+        free: Vec<vk::Fence>,
+    },
 }
 
 impl Fence {
-    fn get_latest(&self, device: &ash::Device) -> Result<crate::FenceValue, crate::DeviceError> {
-        let mut max_value = self.last_completed;
-        for &(value, raw) in self.active.iter() {
+    fn check_active(
+        device: &ash::Device,
+        mut max_value: crate::FenceValue,
+        active: &[(crate::FenceValue, vk::Fence)],
+    ) -> Result<crate::FenceValue, crate::DeviceError> {
+        for &(value, raw) in active.iter() {
             unsafe {
                 if value > max_value && device.get_fence_status(raw)? {
                     max_value = value;
@@ -369,21 +382,52 @@ impl Fence {
         Ok(max_value)
     }
 
+    fn get_latest(
+        &self,
+        device: &ash::Device,
+        extension: Option<&ExtensionFn<khr::TimelineSemaphore>>,
+    ) -> Result<crate::FenceValue, crate::DeviceError> {
+        match *self {
+            Self::TimelineSemaphore(raw) => unsafe {
+                Ok(match *extension.unwrap() {
+                    ExtensionFn::Extension(ref ext) => {
+                        ext.get_semaphore_counter_value(device.handle(), raw)?
+                    }
+                    ExtensionFn::Promoted => device.get_semaphore_counter_value(raw)?,
+                })
+            },
+            Self::FencePool {
+                last_completed,
+                ref active,
+                free: _,
+            } => Self::check_active(device, last_completed, active),
+        }
+    }
+
     fn maintain(&mut self, device: &ash::Device) -> Result<(), crate::DeviceError> {
-        let latest = self.get_latest(device)?;
-        let base_free = self.free.len();
-        for &(value, raw) in self.active.iter() {
-            if value <= latest {
-                self.free.push(raw);
+        match *self {
+            Self::TimelineSemaphore(_) => {}
+            Self::FencePool {
+                ref mut last_completed,
+                ref mut active,
+                ref mut free,
+            } => {
+                let latest = Self::check_active(device, *last_completed, active)?;
+                let base_free = free.len();
+                for &(value, raw) in active.iter() {
+                    if value <= latest {
+                        free.push(raw);
+                    }
+                }
+                if free.len() != base_free {
+                    active.retain(|&(value, _)| value > latest);
+                    unsafe {
+                        device.reset_fences(&free[base_free..])?;
+                    }
+                }
+                *last_completed = latest;
             }
         }
-        if self.free.len() != base_free {
-            self.active.retain(|&(value, _)| value > latest);
-            unsafe {
-                device.reset_fences(&self.free[base_free..])?;
-            }
-        }
-        self.last_completed = latest;
         Ok(())
     }
 }
@@ -394,35 +438,49 @@ impl crate::Queue<Api> for Queue {
         command_buffers: &[&CommandBuffer],
         signal_fence: Option<(&mut Fence, crate::FenceValue)>,
     ) -> Result<(), crate::DeviceError> {
-        let fence_raw = match signal_fence {
-            Some((fence, value)) => {
-                fence.maintain(&self.device.raw)?;
-                let raw = match fence.free.pop() {
-                    Some(raw) => raw,
-                    None => {
-                        let vk_info = vk::FenceCreateInfo::builder().build();
-                        self.device.raw.create_fence(&vk_info, None)?
-                    }
-                };
-                fence.active.push((value, raw));
-                raw
-            }
-            None => vk::Fence::null(),
-        };
-
         let vk_cmd_buffers = command_buffers
             .iter()
             .map(|cmd| cmd.raw)
             .collect::<Vec<_>>();
-        //TODO: semaphores
 
-        let vk_info = vk::SubmitInfo::builder()
-            .command_buffers(&vk_cmd_buffers)
-            .build();
+        let mut vk_info = vk::SubmitInfo::builder().command_buffers(&vk_cmd_buffers);
+
+        let mut fence_raw = vk::Fence::null();
+        let mut vk_timeline_info;
+        let signal_values;
+        let signal_semaphores;
+        if let Some((fence, value)) = signal_fence {
+            fence.maintain(&self.device.raw)?;
+            match *fence {
+                Fence::TimelineSemaphore(raw) => {
+                    signal_values = [value];
+                    signal_semaphores = [raw];
+                    vk_timeline_info = vk::TimelineSemaphoreSubmitInfo::builder()
+                        .signal_semaphore_values(&signal_values);
+                    vk_info = vk_info
+                        .signal_semaphores(&signal_semaphores)
+                        .push_next(&mut vk_timeline_info);
+                }
+                Fence::FencePool {
+                    ref mut active,
+                    ref mut free,
+                    ..
+                } => {
+                    fence_raw = match free.pop() {
+                        Some(raw) => raw,
+                        None => {
+                            let vk_info = vk::FenceCreateInfo::builder().build();
+                            self.device.raw.create_fence(&vk_info, None)?
+                        }
+                    };
+                    active.push((value, fence_raw));
+                }
+            }
+        }
 
         self.device
             .raw
-            .queue_submit(self.raw, &[vk_info], fence_raw)?;
+            .queue_submit(self.raw, &[vk_info.build()], fence_raw)?;
         Ok(())
     }