From 845fdc7027b6e7c28b07c856c713afcf57aede9b Mon Sep 17 00:00:00 2001 From: Jedjoud10 Date: Thu, 12 Oct 2023 19:25:28 -0400 Subject: [PATCH] Fixes #70 and #61 --- src/allocator/scratch_allocator.rs | 36 +++++++++++++++--------------- src/core/app_info.rs | 12 ++++++++-- src/core/physical_device.rs | 12 +++++++--- tests/allocator.rs | 22 +++++++++--------- tests/buffer.rs | 10 ++++----- tests/framework/mod.rs | 2 ++ tests/init.rs | 4 ++-- 7 files changed, 57 insertions(+), 41 deletions(-) diff --git a/src/allocator/scratch_allocator.rs b/src/allocator/scratch_allocator.rs index d882c9d..cc16346 100644 --- a/src/allocator/scratch_allocator.rs +++ b/src/allocator/scratch_allocator.rs @@ -71,6 +71,7 @@ pub struct ScratchAllocator { allocator: A, buffers: Vec>, current_buffer: usize, + largest_block_size: vk::DeviceSize, local_offset: vk::DeviceSize, chunk_size: vk::DeviceSize, alignment: vk::DeviceSize, @@ -126,6 +127,10 @@ impl ScratchAllocator { local_offset: 0, chunk_size, current_buffer: 0, + + // we already pre-allocate a block so this cannot be 0 + largest_block_size: chunk_size, + alignment, device, allocator: allocator.clone(), @@ -165,6 +170,11 @@ impl ScratchAllocator { let whole_buffer_size = size.max(self.chunk_size); let whole_buffer_size = ((whole_buffer_size as f32) / (self.alignment as f32)).ceil() as u64 * self.alignment; + // Per #70, allocate a block twice the size the largest current block (if possible) + // In case the allocation is bigger than the twice the largest block size, simply use the largest one + let whole_buffer_size = (self.largest_block_size * 2).max(whole_buffer_size); + self.largest_block_size = self.largest_block_size.max(whole_buffer_size); + // Create a new chunked buffer with the chunk size let buffer = Buffer::new(self.device.clone(), &mut self.allocator, whole_buffer_size, MemoryType::CpuToGpu)?; if !buffer.is_mapped() { @@ -213,29 +223,19 @@ impl ScratchAllocator { /// Ok(()) /// } /// ``` - pub unsafe fn reset(&mut self, compressed_sized: Option) -> Result<()> { + pub unsafe fn reset(&mut self) -> Result<()> { // Compressed size after reset when we have multiple buffers if self.buffers.len() > 1 { - let compressed_size = compressed_sized.map(|size| { - ((size as f32) / (self.alignment as f32)).ceil() as u64 * self.alignment - }).unwrap_or_else(|| { - // We know that the sizes of the buffers is always aligned, so we shouldn't need to re-align - self.buffers.iter().map(|buf| buf.size()).sum() - }); - - // Create a new buffer that should contain *all* allocations during the frame - let buffer = Buffer::new(self.device.clone(), &mut self.allocator, compressed_size, MemoryType::CpuToGpu)?; - if !buffer.is_mapped() { - anyhow::bail!(Error::UnmappableBuffer); - } - + // Find the largest block (could optimize this by storing index instead) + let index = self.buffers.iter().position(|buffer| buffer.size() == self.largest_block_size).unwrap(); + let buffer = self.buffers.remove(index); self.buffers.clear(); self.buffers.push(buffer); } - + self.current_buffer = 0; self.local_offset = 0; - + return Ok(()); } } @@ -244,6 +244,6 @@ impl Poolable for ScratchAllocator { type Key = (); fn on_release(&mut self) { - unsafe { self.reset(None).unwrap() } + unsafe { self.reset().unwrap() } } -} +} \ No newline at end of file diff --git a/src/core/app_info.rs b/src/core/app_info.rs index f900ff5..b79ef7a 100644 --- a/src/core/app_info.rs +++ b/src/core/app_info.rs @@ -61,9 +61,11 @@ pub struct QueueRequest { /// // Enable an optional Vulkan feature. /// requirements.features.sampler_anisotropy = vk::TRUE; /// ``` -#[derive(Default, Debug)] +#[derive(Derivative)] +#[derivative(Debug)] +#[derivative(Default)] pub struct GPURequirements { -/// Whether a dedicated GPU is required. Setting this to true will discard integrated GPUs. + /// Whether a dedicated GPU is required. Setting this to true will discard integrated GPUs. pub dedicated: bool, /// Minimum amount of video memory required, in bytes. Note that this might count shared memory if RAM is shared. pub min_video_memory: usize, @@ -86,6 +88,12 @@ pub struct GPURequirements { pub features_1_3: vk::PhysicalDeviceVulkan13Features, /// Vulkan device extensions that should be present and enabled. pub device_extensions: Vec, + /// Callback function that will be used to select between multiple physical devices + /// The returned integer is a theoretical score for each physical device + /// Physical Devices with higher "scores" will be prioritized + #[derivative(Debug = "ignore")] + #[derivative(Default(value = "Box::new(|_| 0)"))] + pub scoring_callback: Box usize>, } /// Extra data that is stored within the AppSettings whenever we want to enable renderable Surfaces diff --git a/src/core/physical_device.rs b/src/core/physical_device.rs index ca50f98..495f93f 100644 --- a/src/core/physical_device.rs +++ b/src/core/physical_device.rs @@ -39,6 +39,7 @@ pub struct PhysicalDevice { impl PhysicalDevice { /// Selects the best available physical device from the given requirements and parameters. + /// Also makes use of the given user callback for selecting between multiple candidates pub fn select( instance: &Instance, surface: Option<&Surface>, @@ -49,9 +50,9 @@ impl PhysicalDevice { return Err(anyhow::Error::from(Error::NoGPU)); } - devices + let mut candidates = devices .iter() - .find_map(|device| -> Option { + .filter_map(|device| -> Option { let mut physical_device = PhysicalDevice { handle: *device, properties: unsafe { instance.get_physical_device_properties(*device) }, @@ -203,7 +204,12 @@ impl PhysicalDevice { trace!("Created new VkPhysicalDevice {:p}", physical_device.handle); Some(physical_device) }) - .ok_or_else(|| anyhow::Error::from(Error::NoGPU)) + .map(|physical_device| ((settings.gpu_requirements.scoring_callback)(&physical_device), physical_device)) + .collect::>(); + + candidates.sort_by_key(|(score, _)| *score); + let (_, chosen) = candidates.remove(0); + Ok(chosen) } /// Get all queue families available on this device. This is different from diff --git a/tests/allocator.rs b/tests/allocator.rs index 59d89d4..6ede885 100644 --- a/tests/allocator.rs +++ b/tests/allocator.rs @@ -8,7 +8,7 @@ mod framework; #[test] pub fn basic_allocator_usage() -> Result<()> { - let context = framework::make_context().expect("Can initialize context."); + let context = framework::make_context().expect("Failed to initialize context."); let mut allocator = context.allocator.clone(); let allocation = allocator.allocate( "allocation", @@ -32,7 +32,7 @@ pub fn basic_allocator_usage() -> Result<()> { #[test] pub fn cpu_to_gpu_is_mappable() -> Result<()> { - let context = framework::make_context().expect("Can initialize context."); + let context = framework::make_context().expect("Failed to initialize context."); let mut allocator = context.allocator.clone(); let allocation = allocator.allocate( "allocation", @@ -55,7 +55,7 @@ const CHUNK_SIZE: u64 = 32768; #[test] pub fn make_scratch_allocator() -> Result<()> { - let mut context = framework::make_context().expect("Can initialize context."); + let mut context = framework::make_context().expect("Failed to initialize context."); let _scratch_allocator = ScratchAllocator::new(context.device.clone(), &mut context.allocator, CHUNK_SIZE)?; @@ -64,7 +64,7 @@ pub fn make_scratch_allocator() -> Result<()> { #[test] pub fn use_scratch_allocator() -> Result<()> { - let mut context = framework::make_context().expect("Can initialize context."); + let mut context = framework::make_context().expect("Failed to initialize context."); let mut scratch_allocator = ScratchAllocator::new(context.device.clone(), &mut context.allocator, CHUNK_SIZE)?; // Try allocating a buffer that should fit in the scratch allocator's memory. @@ -75,7 +75,7 @@ pub fn use_scratch_allocator() -> Result<()> { #[test] pub fn use_entire_scratch_allocator() -> Result<()> { // Try allocating the entire scratch allocator's memory for a single buffer - let mut context = framework::make_context().expect("Can initialize context."); + let mut context = framework::make_context().expect("Failed to initialize context."); let mut scratch_allocator = ScratchAllocator::new(context.device.clone(), &mut context.allocator, CHUNK_SIZE)?; let _buffer = scratch_allocator.allocate(1024 as u64)?; @@ -84,7 +84,7 @@ pub fn use_entire_scratch_allocator() -> Result<()> { #[test] pub fn scratch_allocator_allocate_new_chunks() -> Result<()> { - let mut context = framework::make_context().expect("Can initialize context."); + let mut context = framework::make_context().expect("Failed to initialize context."); let mut scratch_allocator = ScratchAllocator::new(context.device.clone(), &mut context.allocator, 1024)?; // First allocate a smaller buffer @@ -96,20 +96,20 @@ pub fn scratch_allocator_allocate_new_chunks() -> Result<()> { #[test] pub fn reset_scratch_allocator() -> Result<()> { - let mut context = framework::make_context().expect("Can initialize context."); + let mut context = framework::make_context().expect("Failed to initialize context."); let mut scratch_allocator = ScratchAllocator::new(context.device.clone(), &mut context.allocator, CHUNK_SIZE)?; // Allocate a first buffer. - let _buffer = scratch_allocator.allocate(800 as u64)?; + let buffer = scratch_allocator.allocate(800 as u64)?; // Now reset it, so we should be able to allocate again - unsafe { scratch_allocator.reset(None)?; } + unsafe { scratch_allocator.reset()?; } let _buffer = scratch_allocator.allocate(800 as u64)?; Ok(()) } #[test] pub fn scratch_allocator_mass_allocate() -> Result<()> { - let mut context = framework::make_context().expect("Can initialize context."); + let mut context = framework::make_context().expect("Failed to initialize context."); let mut scratch_allocator = ScratchAllocator::new(context.device.clone(), &mut context.allocator, CHUNK_SIZE)?; @@ -128,7 +128,7 @@ pub fn scratch_allocator_mass_allocate() -> Result<()> { } // Now reset it, so we should be able to allocate again - unsafe { scratch_allocator.reset(None)?; } + unsafe { scratch_allocator.reset()?; } } Ok(()) diff --git a/tests/buffer.rs b/tests/buffer.rs index e390e15..e9cb5f0 100644 --- a/tests/buffer.rs +++ b/tests/buffer.rs @@ -8,7 +8,7 @@ mod framework; #[test] pub fn alloc_buffer() -> Result<()> { - let mut context = framework::make_context().expect("Can initialize context."); + let mut context = framework::make_context().expect("Failed to initialize context."); const ALLOC_SIZE: u64 = 1024u64; @@ -31,7 +31,7 @@ pub fn alloc_buffer() -> Result<()> { #[test] pub fn alloc_aligned_buffer() -> Result<()> { - let mut context = framework::make_context().expect("Can initialize context."); + let mut context = framework::make_context().expect("Failed to initialize context."); const ALLOC_SIZE: u64 = 1000u64; const ALIGN: u64 = 128; @@ -57,7 +57,7 @@ pub fn alloc_aligned_buffer() -> Result<()> { #[test] pub fn buffer_view_full() -> Result<()> { - let mut context = framework::make_context().expect("Can initialize context."); + let mut context = framework::make_context().expect("Failed to initialize context."); const ALLOC_SIZE: u64 = 1024u64; @@ -83,7 +83,7 @@ pub fn buffer_view_full() -> Result<()> { #[test] pub fn buffer_view() -> Result<()> { - let mut context = framework::make_context().expect("Can initialize context."); + let mut context = framework::make_context().expect("Failed to initialize context."); const ALLOC_SIZE: u64 = 1024u64; @@ -115,7 +115,7 @@ pub fn buffer_view() -> Result<()> { #[test] pub fn invalid_buffer_view() -> Result<()> { - let mut context = framework::make_context().expect("Can initialize context."); + let mut context = framework::make_context().expect("Failed to initialize context."); const ALLOC_SIZE: u64 = 1024u64; diff --git a/tests/framework/mod.rs b/tests/framework/mod.rs index 4406f5a..e9259fd 100644 --- a/tests/framework/mod.rs +++ b/tests/framework/mod.rs @@ -47,6 +47,7 @@ pub fn make_context_with_queues( features_1_2: Default::default(), features_1_3: Default::default(), device_extensions: vec![], + scoring_callback: Box::new(|_| 0) }) .build(); let (instance, phys_device, None, device, allocator, pool, exec, None, None) = @@ -87,6 +88,7 @@ pub fn make_context_with_settings< features_1_2: Default::default(), features_1_3: Default::default(), device_extensions: vec![], + scoring_callback: Box::new(|_| 0) }); let settings = callback(builder).build(); diff --git a/tests/init.rs b/tests/init.rs index af15277..92da6b5 100644 --- a/tests/init.rs +++ b/tests/init.rs @@ -55,7 +55,7 @@ pub fn requesting_raytracing_does_not_fail() -> Result<()> { #[test] pub fn vulkan_loaded() -> Result<()> { - let context = framework::make_context().expect("Can initialize context."); + let context = framework::make_context().expect("Failed to initialize context."); let handle = context.instance.handle(); assert_ne!(handle.as_raw(), 0, "VkInstance handle should not be zero"); let loader = unsafe { context.instance.loader() }; @@ -65,7 +65,7 @@ pub fn vulkan_loaded() -> Result<()> { #[test] pub fn valid_device() -> Result<()> { - let context = framework::make_context().expect("Can initialize context."); + let context = framework::make_context().expect("Failed to initialize context."); let handle = unsafe { context.device.handle() }; assert_ne!(handle.handle().as_raw(), 0, "VkDevice handle should not be zero"); // Also try a vulkan function call on it to make sure it is loaded properly