pipeline cache with properly managed descruction

crates/renderer/src/device.rs

@@ -110,11 +110,15 @@ pub struct DeviceInner {
     #[allow(dead_code)]
     pub(crate) enabled_extensions: Vec<&'static CStr>,
 
-    pub(crate) pipeline_cache: PipelineCache,
-
     _drop: DeviceDrop,
 }
 
+impl AsRef<DeviceInner> for DeviceInner {
+    fn as_ref(&self) -> &DeviceInner {
+        self
+    }
+}
+
 impl core::fmt::Debug for DeviceInner {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
         f.debug_struct("DeviceInner")
@@ -400,7 +404,6 @@ impl PhysicalDeviceInfo {
             raw: device.clone(),
             alloc2: Mutex::new(alloc2),
             instance: instance.clone(),
-            pipeline_cache: PipelineCache::new(&device, &self)?,
             adapter: self,
             queues: device_queues,
             device_extensions,
@@ -446,6 +449,13 @@ pub(crate) struct DevicePools {
     pub(crate) fences: Arc<Pool<vk::Fence>>,
     pub(crate) binary_semaphores: Pool<BinarySemaphore>,
     pub(crate) timeline_semaphores: Pool<TimelineSemaphore>,
+    pub(crate) pipeline_cache: asdf::DeviceObject<PipelineCache, Arc<DeviceInner>>,
+}
+
+impl AsRef<DevicePools> for DevicePools {
+    fn as_ref(&self) -> &DevicePools {
+        self
+    }
 }
 
 impl DevicePools {
@@ -453,7 +463,11 @@ impl DevicePools {
         Self {
             fences: Arc::new(Pool::new(device.clone())),
             binary_semaphores: Pool::new(device.clone()),
-            timeline_semaphores: Pool::new(device),
+            timeline_semaphores: Pool::new(device.clone()),
+            pipeline_cache: asdf::DeviceObject::new(
+                device.clone(),
+                PipelineCache::new(&device.raw, &device.adapter).unwrap(),
+            ),
         }
     }
 }
@@ -740,7 +754,7 @@ impl<T: DeviceHandle> DeviceObject<T> {
     pub fn name(&self) -> Option<&str> {
         #[cfg(debug_assertions)]
         {
-            self.name.as_deref().map(|cow| cow.as_ref())
+            self.name.as_deref()
         }
         #[cfg(not(debug_assertions))]
         {
@@ -833,23 +847,20 @@ impl<T> AsRef<Pool<T>> for Pool<T> {
     }
 }
 
-pub type PoolObject<T, U: AsRef<Pool<T>>> = asdf::ExternallyManagedObject<T, U>;
+pub type PoolObject<T, U = Arc<Pool<T>>> = asdf::ExternallyManagedObject<T, U>;
 
-impl<'a, T: Pooled + asdf::traits::ExternallyManagedObject<&'a Pool<T>> + 'a> Pool<T> {
+impl<T: Pooled> Pool<T> {
-    pub fn get(&'a self) -> Result<PoolObject<T, &'a Pool<T>>> {
+    pub fn get(&self) -> Result<T> {
         let item = if let Some(item) = self.pool.lock().pop() {
             item
         } else {
             T::create_from_pool(self)?
         };
 
-        Ok(asdf::ExternallyManagedObject::new(item, self))
+        Ok(item)
     }
 
-    pub fn get_debug_named(
+    pub fn get_debug_named(&self, name: Option<impl Into<Cow<'static, str>>>) -> Result<T>
-        &'a self,
-        name: Option<impl Into<Cow<'static, str>>>,
-    ) -> Result<PoolObject<T, &'a Pool<T>>>
     where
         T: asdf::traits::DebugNameable,
     {
@@ -933,10 +944,7 @@ pub(crate) mod asdf {
 
     use ash::vk;
 
-    use crate::{
+    use crate::{device::DeviceInner, util::DebugName};
-        device::{DeviceInner, DevicePools, GpuAllocation},
-        util::DebugName,
-    };
 
     pub mod traits {
         /// A trait describing an object owned by some manager-type, which is
@@ -954,6 +962,7 @@ pub(crate) mod asdf {
     }
 
     /// Wrapper for types which are owned by another type `O`, which is responsible for destruction.
+    #[derive(Debug)]
     pub struct ExternallyManagedObject<T: traits::ExternallyManagedObject<O>, O> {
         inner: ManuallyDrop<T>,
         owner: O,
@@ -1034,11 +1043,13 @@ pub(crate) mod asdf {
     }
 
     /// A wrapper for vulkan types which are owned by the device, taking care of destruction.
+    #[derive(Debug)]
     pub struct DeviceObject<
         T: traits::ExternallyManagedObject<O>,
         O: AsRef<super::DeviceInner> = Arc<super::DeviceInner>,
     > {
         inner: ExternallyManagedObject<T, O>,
+        #[allow(dead_code)]
         name: Option<DebugName>,
     }
 
@@ -1047,7 +1058,7 @@ pub(crate) mod asdf {
         O: AsRef<super::DeviceInner>,
     > DeviceObject<T, O>
     {
-        fn new_debug_named(owner: O, inner: T, name: Option<impl Into<DebugName>>) -> Self {
+        pub fn new_debug_named(owner: O, inner: T, name: Option<impl Into<DebugName>>) -> Self {
             let name = name.map(Into::into);
             if let Some(ref name) = name {
                 traits::DebugNameable::debug_name(&inner, owner.as_ref(), name);
@@ -1060,12 +1071,12 @@ pub(crate) mod asdf {
     }
 
     impl<T: traits::ExternallyManagedObject<O>, O: AsRef<super::DeviceInner>> DeviceObject<T, O> {
-        fn new(owner: O, inner: T) -> Self {
+        pub fn new(owner: O, inner: T) -> Self {
             let inner = ExternallyManagedObject::new(inner, owner);
 
             Self { inner, name: None }
         }
-        fn device(&self) -> &O {
+        pub fn device(&self) -> &O {
             self.inner.owner()
         }
     }
@@ -1109,25 +1120,6 @@ pub(crate) mod asdf {
             }
         }
 
-        impl traits::ExternallyManagedObject<DevicePools> for super::Semaphore {
-            unsafe fn destroy(self, owner: &DevicePools) {
-                match self {
-                    super::Semaphore::Binary(semaphore) => owner
-                        .binary_semaphores
-                        .push(crate::sync::BinarySemaphore(semaphore)),
-                    super::Semaphore::Timeline(semaphore) => owner
-                        .timeline_semaphores
-                        .push(crate::sync::TimelineSemaphore(semaphore)),
-                }
-            }
-        }
-
-        impl AsRef<DeviceInner> for DeviceInner {
-            fn as_ref(&self) -> &DeviceInner {
-                self
-            }
-        }
-
         impl<T> traits::DebugNameable for T
         where
             T: vk::Handle + Copy,
@@ -1140,41 +1132,33 @@ pub(crate) mod asdf {
         }
     }
 
-    enum Semaphore {
+    #[allow(dead_code)]
-        Binary(vk::Semaphore),
+    #[cfg(test)]
-        Timeline(vk::Semaphore),
-    }
-
-    fn summon<T>() -> T {
-        unimplemented!()
-    }
-
     fn asdf() {
-        let inner_ref: DeviceObject<vk::Semaphore, &DeviceInner> = DeviceObject::new_debug_named(
+        use crate::device::{DevicePools, GpuAllocation};
+        fn summon<T>() -> T {
+            unimplemented!()
+        }
+
+        let _inner_ref: DeviceObject<vk::Semaphore, &DeviceInner> = DeviceObject::new_debug_named(
             summon::<&DeviceInner>(),
             summon::<vk::Semaphore>(),
             Some("my semaphore"),
         );
 
-        let device_owned: DeviceObject<vk::Semaphore, super::Device> =
+        let _device_owned: DeviceObject<vk::Semaphore, super::Device> =
             DeviceObject::new_debug_named(
                 summon::<super::Device>(),
                 summon::<vk::Semaphore>(),
                 Some("my other semaphore"),
             );
 
-        let allocation: DeviceObject<GpuAllocation, Arc<super::DeviceInner>> = DeviceObject::new(
+        let _allocation: DeviceObject<GpuAllocation, Arc<super::DeviceInner>> = DeviceObject::new(
             summon::<Arc<super::DeviceInner>>(),
             summon::<GpuAllocation>(),
         );
 
-        let pool_owned: ExternallyManagedObject<vk::Semaphore, DevicePools> =
+        let _pool_owned: ExternallyManagedObject<vk::Semaphore, DevicePools> =
             ExternallyManagedObject::new(summon::<vk::Semaphore>(), summon::<DevicePools>());
-
-        let enum_semaphore_pooled: ExternallyManagedObject<Semaphore, DevicePools> =
-            ExternallyManagedObject::new(
-                Semaphore::Binary(summon::<vk::Semaphore>()),
-                summon::<DevicePools>(),
-            );
     }
 }

crates/renderer/src/pipeline.rs

@@ -503,7 +503,7 @@ impl Pipeline {
             device
                 .dev()
                 .create_compute_pipelines(
-                    device.pipeline_cache.raw,
+                    device.pools.pipeline_cache.raw,
                     core::slice::from_ref(info),
                     None,
                 )
@@ -575,27 +575,23 @@ impl Pipeline {
         });
 
         let multisample = desc.multisample.map(|state| {
-            let info = vk::PipelineMultisampleStateCreateInfo::default()
+            vk::PipelineMultisampleStateCreateInfo::default()
                 .flags(state.flags)
                 .min_sample_shading(state.min_sample_shading)
                 .rasterization_samples(state.rasterization_samples)
                 .sample_mask(state.sample_mask)
                 .sample_shading_enable(state.sample_shading_enable)
                 .alpha_to_coverage_enable(state.alpha_to_coverage_enable)
-                .alpha_to_one_enable(state.alpha_to_one_enable);
+                .alpha_to_one_enable(state.alpha_to_one_enable)
-
-            info
         });
 
         let color_blend = desc.color_blend.map(|state| {
-            let info = vk::PipelineColorBlendStateCreateInfo::default()
+            vk::PipelineColorBlendStateCreateInfo::default()
                 .flags(state.flags)
                 .attachments(state.attachments)
                 .blend_constants(state.blend_constants)
                 .logic_op(state.logic_op.unwrap_or(Default::default()))
-                .logic_op_enable(state.logic_op.is_some());
+                .logic_op_enable(state.logic_op.is_some())
-
-            info
         });
 
         let depth_stencil = desc.depth_stencil.map(|state| {
@@ -625,20 +621,16 @@ impl Pipeline {
         });
 
         let dynamic = desc.dynamic.map(|state| {
-            let info = vk::PipelineDynamicStateCreateInfo::default()
+            vk::PipelineDynamicStateCreateInfo::default()
                 .flags(state.flags)
-                .dynamic_states(state.dynamic_states);
+                .dynamic_states(state.dynamic_states)
-
-            info
         });
 
         let mut rendering = desc.rendering.map(|state| {
-            let info = vk::PipelineRenderingCreateInfo::default()
+            vk::PipelineRenderingCreateInfo::default()
                 .color_attachment_formats(state.color_formats)
                 .depth_attachment_format(state.depth_format.unwrap_or_default())
-                .stencil_attachment_format(state.stencil_format.unwrap_or_default());
+                .stencil_attachment_format(state.stencil_format.unwrap_or_default())
-
-            info
         });
 
         fn option_to_ptr<T>(option: &Option<T>) -> *const T {
@@ -679,7 +671,7 @@ impl Pipeline {
             device
                 .dev()
                 .create_graphics_pipelines(
-                    device.pipeline_cache.raw,
+                    device.pools.pipeline_cache.raw,
                     core::slice::from_ref(&info),
                     None,
                 )
@@ -704,17 +696,33 @@ impl Pipeline {
 }
 
 pub(crate) mod pipeline_cache {
+    use std::sync::Arc;
+
     use ash::vk;
 
     use ash::Device;
 
     use crate::PhysicalDeviceInfo;
+    use crate::device::DeviceInner;
 
+    #[derive(Debug)]
     pub struct PipelineCache {
+        #[allow(dead_code)]
         key: u128,
         pub(crate) raw: vk::PipelineCache,
     }
 
+    impl crate::device::asdf::traits::ExternallyManagedObject<Arc<DeviceInner>> for PipelineCache {
+        unsafe fn destroy(self, owner: &Arc<DeviceInner>) {
+            if let Ok(data) = self.export(&owner.raw) {
+                _ = Self::write_to_disk(self.key, &data).inspect_err(|err| {
+                    tracing::error!("failed to write pipeline cache to disk: {err}");
+                });
+            }
+            unsafe { owner.raw.destroy_pipeline_cache(self.raw, None) };
+        }
+    }
+
     impl PipelineCache {
         const MAGIC: [u8; 4] = *b"VYPC";
         const KEY_VERSION: u32 = 1;
@@ -785,7 +793,7 @@ pub(crate) mod pipeline_cache {
             });
 
             let info = vk::PipelineCacheCreateInfo::default()
-                .flags(vk::PipelineCacheCreateFlags::EXTERNALLY_SYNCHRONIZED)
+                // .flags(vk::PipelineCacheCreateFlags::EXTERNALLY_SYNCHRONIZED)
                 .initial_data(data.as_deref().unwrap_or_default());
 
             let cache = unsafe { device.create_pipeline_cache(&info, None)? };

crates/renderer/src/sync.rs

@@ -1,5 +1,3 @@
-#[cfg(debug_assertions)]
-use std::borrow::Cow;
 use std::{
     future::Future,
     marker::PhantomData,
@@ -7,7 +5,10 @@ use std::{
     time::Duration,
 };
 
-use crate::device::{DeviceObject, Pool, PoolObject, Pooled};
+use crate::device::{
+    DevicePools, Pool, PoolObject, Pooled,
+    asdf::{DeviceObject, traits::ExternallyManagedObject as ExternallyManagedObjectTrait},
+};
 use crate::{Result, device::DeviceInner};
 
 use super::Device;
@@ -175,16 +176,20 @@ impl Pooled for vk::Fence {
     }
 }
 
-impl<T> crate::device::asdf::traits::ExternallyManagedObject<T> for vk::Fence
+impl ExternallyManagedObjectTrait<Arc<Pool<vk::Fence>>> for vk::Fence {
-where
+    unsafe fn destroy(self, pool: &Arc<Pool<vk::Fence>>) {
-    T: AsRef<Pool<vk::Fence>>,
-{
-    unsafe fn destroy(self, owner: &T) {
-        let pool = owner.as_ref();
         pool.push(self);
     }
 }
 
+impl ExternallyManagedObjectTrait<Arc<DeviceInner>> for vk::Fence {
+    unsafe fn destroy(self, device: &Arc<DeviceInner>) {
+        unsafe {
+            device.raw.destroy_fence(self, None);
+        }
+    }
+}
+
 impl std::fmt::Debug for Fence {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
         f.debug_struct("Fence").field("fence", &self.raw()).finish()
@@ -199,13 +204,15 @@ impl Fence {
                 .create_fence(&vk::FenceCreateInfo::default(), None)?
         };
         Ok(Self::Dedicated {
-            fence: DeviceObject::new(fence, device, name.map(Into::into)),
+            fence: DeviceObject::new_debug_named(device.shared, fence, name),
         })
     }
     pub fn from_pool(pool: &Arc<Pool<vk::Fence>>, name: Option<&'static str>) -> Result<Fence> {
-        let fence = pool.get_debug_named(name)?.map_owner(|_| pool.clone());
+        let fence = pool.get_debug_named(name)?;
 
-        Ok(Self::Pooled { fence })
+        Ok(Self::Pooled {
+            fence: PoolObject::new(fence, pool.clone()),
+        })
     }
 
     pub fn raw(&self) -> vk::Fence {
@@ -217,7 +224,7 @@ impl Fence {
 
     fn device(&self) -> &Arc<DeviceInner> {
         match self {
-            Fence::Dedicated { fence } => &fence.device().shared,
+            Fence::Dedicated { fence } => &fence.device(),
             Fence::Pooled { fence } => &fence.owner().device,
         }
     }
@@ -262,23 +269,81 @@ pub enum SemaphoreType {
     Timeline(u64),
 }
 
+#[derive(Debug, Clone, Copy)]
+pub enum SemaphoreInner {
+    Binary(vk::Semaphore),
+    Timeline(vk::Semaphore),
+}
+
+impl ExternallyManagedObjectTrait<Arc<DevicePools>> for SemaphoreInner {
+    unsafe fn destroy(self, owner: &Arc<DevicePools>) {
+        match self {
+            SemaphoreInner::Binary(semaphore) => {
+                owner.binary_semaphores.push(BinarySemaphore(semaphore))
+            }
+            SemaphoreInner::Timeline(semaphore) => {
+                owner.timeline_semaphores.push(TimelineSemaphore(semaphore))
+            }
+        }
+    }
+}
+
+impl ExternallyManagedObjectTrait<Arc<DeviceInner>> for SemaphoreInner {
+    unsafe fn destroy(self, owner: &Arc<DeviceInner>) {
+        match self {
+            SemaphoreInner::Binary(semaphore) | SemaphoreInner::Timeline(semaphore) => {
+                unsafe { owner.raw.destroy_semaphore(semaphore, None) };
+            }
+        }
+    }
+}
+
+impl crate::device::asdf::traits::DebugNameable for SemaphoreInner {
+    fn debug_name(&self, device: &DeviceInner, name: &str) {
+        unsafe {
+            device.debug_name_object(self.raw(), name);
+        }
+    }
+}
+
+impl SemaphoreInner {
+    pub fn raw(&self) -> vk::Semaphore {
+        match self {
+            SemaphoreInner::Binary(semaphore) | SemaphoreInner::Timeline(semaphore) => *semaphore,
+        }
+    }
+    pub fn semaphore_type(&self) -> SemaphoreType {
+        match self {
+            SemaphoreInner::Binary(_) => SemaphoreType::Binary,
+            SemaphoreInner::Timeline(_) => SemaphoreType::Timeline(!0),
+        }
+    }
+}
+
+impl From<BinarySemaphore> for SemaphoreInner {
+    fn from(value: BinarySemaphore) -> Self {
+        SemaphoreInner::Binary(value.0)
+    }
+}
+impl From<TimelineSemaphore> for SemaphoreInner {
+    fn from(value: TimelineSemaphore) -> Self {
+        SemaphoreInner::Timeline(value.0)
+    }
+}
+
 pub enum Semaphore {
     Dedicated {
-        semaphore_type: SemaphoreType,
+        semaphore: DeviceObject<SemaphoreInner>,
-        semaphore: DeviceObject<vk::Semaphore>,
     },
     Pooled {
-        semaphore_type: SemaphoreType,
+        #[allow(private_interfaces)]
-        semaphore: vk::Semaphore,
+        semaphore: PoolObject<SemaphoreInner, Arc<DevicePools>>,
-        device: Device,
-        #[cfg(debug_assertions)]
-        name: Option<Cow<'static, str>>,
     },
 }
 
-#[derive(Debug, Clone)]
+#[derive(Debug, Clone, Copy)]
 pub(crate) struct BinarySemaphore(pub(crate) vk::Semaphore);
-#[derive(Debug, Clone)]
+#[derive(Debug, Clone, Copy)]
 pub(crate) struct TimelineSemaphore(pub(crate) vk::Semaphore);
 
 // This is just so that ash can name these semaphore newtypes
@@ -327,36 +392,6 @@ impl Pooled for TimelineSemaphore {
     }
 }
 
-impl Drop for Semaphore {
-    fn drop(&mut self) {
-        if let Semaphore::Pooled {
-            device,
-            semaphore_type,
-            semaphore,
-            name,
-        } = self
-        {
-            #[cfg(debug_assertions)]
-            if name.is_some() {
-                // reset the name to avoid confusion in case this semaphore is re-used
-                unsafe { device.debug_name_object(*semaphore, "") };
-            }
-            match semaphore_type {
-                SemaphoreType::Binary => device
-                    .pools
-                    .binary_semaphores
-                    .push(BinarySemaphore(*semaphore)),
-                SemaphoreType::Timeline(_) => {
-                    device
-                        .pools
-                        .timeline_semaphores
-                        .push(TimelineSemaphore(*semaphore));
-                }
-            }
-        }
-    }
-}
-
 impl Semaphore {
     pub fn new_dedicated(
         device: Device,
@@ -377,9 +412,13 @@ impl Semaphore {
         let create_info = vk::SemaphoreCreateInfo::default().push_next(&mut type_info);
         let inner = unsafe { device.dev().create_semaphore(&create_info, None)? };
 
+        let inner = match semaphore_type {
+            SemaphoreType::Binary => SemaphoreInner::Binary(inner),
+            SemaphoreType::Timeline(_) => SemaphoreInner::Timeline(inner),
+        };
+
         Ok(Self::Dedicated {
-            semaphore_type,
+            semaphore: DeviceObject::new_debug_named(device.shared, inner, name),
-            semaphore: DeviceObject::new(inner, device, name.map(Into::into)),
         })
     }
 
@@ -390,48 +429,35 @@ impl Semaphore {
     ) -> Result<Self> {
         let semaphore = match semaphore_type {
             SemaphoreType::Binary => {
-                if let Some(semaphore) = device.pools.binary_semaphores.pop() {
+                let semaphore: SemaphoreInner =
-                    semaphore.0
+                    device.pools.binary_semaphores.get_debug_named(name)?.into();
-                } else {
+                PoolObject::new(semaphore, device.pools)
-                    let mut type_info = vk::SemaphoreTypeCreateInfo::default()
-                        .semaphore_type(vk::SemaphoreType::BINARY);
-                    let create_info = vk::SemaphoreCreateInfo::default().push_next(&mut type_info);
-                    unsafe { device.raw.create_semaphore(&create_info, None)? }
-                }
             }
             SemaphoreType::Timeline(value) => {
-                if let Some(semaphore) = device.pools.binary_semaphores.pop() {
+                let semaphore: SemaphoreInner = device
-                    semaphore.0
+                    .pools
-                } else {
+                    .timeline_semaphores
-                    let mut type_info = vk::SemaphoreTypeCreateInfo::default()
+                    .get_debug_named(name)?
-                        .semaphore_type(vk::SemaphoreType::TIMELINE)
+                    .into();
-                        .initial_value(value);
+
-                    let create_info = vk::SemaphoreCreateInfo::default().push_next(&mut type_info);
+                let info = vk::SemaphoreSignalInfo::default()
-                    unsafe { device.raw.create_semaphore(&create_info, None)? }
+                    .semaphore(semaphore.raw())
+                    .value(value);
+                unsafe {
+                    device.raw.signal_semaphore(&info)?;
                 }
+
+                PoolObject::new(semaphore, device.pools)
             }
         };
 
-        #[cfg(debug_assertions)]
+        Ok(Self::Pooled { semaphore })
-        if let Some(name) = name {
-            unsafe {
-                device.debug_name_object(semaphore, name);
-            }
-        }
-
-        Ok(Self::Pooled {
-            semaphore_type,
-            semaphore,
-            device,
-            #[cfg(debug_assertions)]
-            name: name.map(Into::into),
-        })
     }
 
     pub fn semaphore(&self) -> vk::Semaphore {
         match self {
-            Semaphore::Dedicated { semaphore, .. } => **semaphore,
+            Semaphore::Dedicated { semaphore, .. } => semaphore.raw(),
-            Semaphore::Pooled { semaphore, .. } => *semaphore,
+            Semaphore::Pooled { semaphore, .. } => semaphore.raw(),
         }
     }
 }