Add support for dedicated allocations and committed resource tracking #148

janie177 · 2022-12-21T16:11:22Z

Changes

This PR adds the option to create dedicated allocations in the Vulkan allocator and CommittedResources in the Dx12 allocator. These types of allocations are managed by the driver, which allows it to do special optimizations (🧙‍♂️ magic).

Vulkan

In the Vulkan allocator, you must now pass an extra bool dedicated_allocation into the allocate function. Vulkan 1.1 is required for this to be available, though there is an extension for Vulkan 1.0 available (which I didn't integrate, but I could do that if we want to support it).

The visualizer now displays whether a memory block is backed by a dedicated allocation or not.

D3D12

In DirectX, dedicated allocations are hidden behind the CreateCommittedResource() call. It is not possible to access these allocations, so I've added a create_resource function to the dx12 allocator which will return a Resource object. You can decide whether you want this to be a Placed or Committed resource, and the rest is handled under the hood for you. When freeing a resource, you have to call the free_resource function on the allocator.

I have added a special section that displays the total amount of committed resources and their total size per memory type to the visualizer.

Testing

I have tested these changes in the dx12 and vulkan backends of the Breda framework. Performance on vulkan and dx12 increased by about 10-fold in our ray-tracing shaders when marking render targets as dedicated resources.

…urces to the dx12 allocator. Add dedicated allocation information to the visualizer.

…e same functions. Add information about committed resources per heap type to the visualizer in d3d12.

…allocator.

…cated allocations to be ignored.

src/d3d12/mod.rs

src/vulkan/mod.rs

src/vulkan/visualizer.rs

src/d3d12/mod.rs

src/d3d12/visualizer.rs

src/vulkan/mod.rs

Remove old heap type information that was leftover from before an API change. Implement several pull request suggestions, and fix a bug that would wrongly display dedicated block information in the vulkan visualizer.

…w an allocation should be managed. Remove the BlockType enum that previously fulfilled the same task under the hood.

…ons.

janie177 · 2022-12-23T13:39:08Z

src/vulkan/mod.rs

+#[derive(Copy, Clone, Debug, Eq, PartialEq)]
+pub enum AllocationScheme {
+ /// This allocation will be for a buffer, and it will be driver managed using vulkans DedicatedAllocation feature.
+ /// The driver will be able to perform optimizations in some cases with this type of allocation.
+ DedicatedBuffer { buffer: vk::Buffer },
+ /// This allocation will be for a image, and it will be driver managed using vulkans DedicatedAllocation feature.
+ /// The driver will be able to perform optimizations in some cases with this type of allocation.
+ DedicatedImage { image: vk::Image },
+ /// This allocation will be managed by the GPU allocator.
+ /// It is possible that the allocation will reside in an allocation block shared with other resources.
+ GpuAllocatorManaged,
+}
+


@MarijnS95 What do you think about this for the user exposed interface?
We can now also correctly pass the buffer and image when required for dedicated allocations. I got rid of the BlockType as that was essentially serving the same purpose, just with less configurability and under the hood.

Interface looks good, though I'd have reworded some of the comments:

Suggested change

#[derive(Copy, Clone, Debug, Eq, PartialEq)]

pub enum AllocationScheme {

/// This allocation will be for a buffer, and it will be driver managed using vulkans DedicatedAllocation feature.

/// The driver will be able to perform optimizations in some cases with this type of allocation.

DedicatedBuffer { buffer: vk::Buffer },

/// This allocation will be for a image, and it will be driver managed using vulkans DedicatedAllocation feature.

/// The driver will be able to perform optimizations in some cases with this type of allocation.

DedicatedImage { image: vk::Image },

/// This allocation will be managed by the GPU allocator.

/// It is possible that the allocation will reside in an allocation block shared with other resources.

GpuAllocatorManaged,

}

#[derive(Copy, Clone, Debug, Eq, PartialEq)]

pub enum AllocationScheme {

/// Perform a dedicated, driver-managed allocation for the given buffer, allowing

/// it to perform optimizations on this type of allocation.

DedicatedBuffer { buffer: vk::Buffer },

/// Perform a dedicated, driver-managed allocation for the given image, allowing

/// it to perform optimizations on this type of allocation.

DedicatedImage { image: vk::Image },

/// The memory for this resource will be allocated and managed by gpu-allocator.

/// It is possible that the allocation will reside in [`vk::Memory`] shared with other resources.

GpuAllocatorManaged,

}

MarijnS95 · 2023-01-12T10:46:49Z

src/d3d12/mod.rs

@@ -743,6 +794,131 @@ impl Allocator {
 }
 }
 }
+
+ /// Create a resource according to the provided parameters.
+ /// Created resources should be freed at the end of their lifetime by calling [`Self::free_resource()`].


Do we need a Drop destructor to call this or print a warning when it was not called?

MarijnS95 · 2023-01-12T10:49:27Z

src/d3d12/mod.rs

+ // Committed resources are implicitly dropped when their refcount reaches 0.
+ // We only have to change the allocation and size tracking.


Well you're explicitly dropping resource with ManuallyDrop right above... It's just that there's no allocation to clean up here.

MarijnS95 · 2023-01-12T10:55:56Z

src/vulkan/mod.rs

 buffer_device_address: bool,
+ allocation_scheme: &AllocationScheme,


This is a cheap Copy type, just pass-by-value. The user will likely ad-hoc create it anyway.

MarijnS95 · 2023-01-12T10:57:02Z

src/vulkan/mod.rs

+ DedicatedBuffer { buffer: vk::Buffer },
+ /// This allocation will be for a image, and it will be driver managed using vulkans DedicatedAllocation feature.
+ /// The driver will be able to perform optimizations in some cases with this type of allocation.
+ DedicatedImage { image: vk::Image },


Also feel free to just use a tuple-variant here, the name of the variant conveys the same name as the only named field here.

MarijnS95 · 2023-01-12T10:58:45Z

src/vulkan/mod.rs

- Box::new(allocator::FreeListAllocator::new(size))
- };
+ let sub_allocator: Box<dyn allocator::SubAllocator> =
+ if !matches!(allocation_scheme, AllocationScheme::GpuAllocatorManaged)


Just use == :)

janie177 added 10 commits December 16, 2022 15:38

Add dedicated allocations to the vulkan allocator, and committed reso…

3657ebb

…urces to the dx12 allocator. Add dedicated allocation information to the visualizer.

Remove duplicate dedicated block info from the visualizer.

e387af9

Handle committed and placed resource creation and destruction with th…

4f78a9a

…e same functions. Add information about committed resources per heap type to the visualizer in d3d12.

Actually draw the imgui windows in the example application for d3d12.

e208616

Add conversion from windows-rs resource to win-api resource.

812d5ba

Allow a clear value to be passed when creating resources in the dx12 …

155f7b6

…allocator.

Formatting, coercion and passing data by reference.

31e830d

Changes to please clippy.

5895df1

Formatting and fix logic flaw in block type creation that caused dedi…

aaf975b

…cated allocations to be ignored.

Remove uselss format.

e2bd716

janie177 requested review from Jasper-Bekkers and MarijnS95 and removed request for Jasper-Bekkers December 21, 2022 16:14

janie177 marked this pull request as ready for review December 21, 2022 16:14

Jasper-Bekkers approved these changes Dec 21, 2022

View reviewed changes

janie177 added 2 commits December 21, 2022 17:30

Update doc comments that were outdated.

84bf6f1

Solve clippy warning that was causing the CI to fail.

54021d3

MarijnS95 reviewed Dec 22, 2022

View reviewed changes

janie177 added 3 commits December 22, 2022 15:26

Some cleanup after clippy refactors.

abcc0cc

Remove old heap type information that was leftover from before an API change. Implement several pull request suggestions, and fix a bug that would wrongly display dedicated block information in the vulkan visualizer.

Add a new enum AllocationScheme which allows the user to determine ho…

5f27ed8

…w an allocation should be managed. Remove the BlockType enum that previously fulfilled the same task under the hood.

Pass vulkan buffers and images not as pointers for dedicated allocati…

0a91320

…ons.

janie177 commented Dec 23, 2022

View reviewed changes

Jasper-Bekkers merged commit 5c422cf into main Jan 12, 2023

MarijnS95 reviewed Jan 12, 2023

View reviewed changes

janie177 mentioned this pull request Jan 16, 2023

Resource ownership cleanup #151

Merged

MarijnS95 deleted the resource-creation-in-gpu-allocator branch January 16, 2023 14:28

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for dedicated allocations and committed resource tracking #148

Add support for dedicated allocations and committed resource tracking #148

janie177 commented Dec 21, 2022 •

edited

Loading

janie177 Dec 23, 2022

MarijnS95 Jan 12, 2023

MarijnS95 Jan 12, 2023

MarijnS95 Jan 12, 2023

MarijnS95 Jan 12, 2023

MarijnS95 Jan 12, 2023

MarijnS95 Jan 12, 2023

		// Committed resources are implicitly dropped when their refcount reaches 0.
		// We only have to change the allocation and size tracking.

		buffer_device_address: bool,
		allocation_scheme: &AllocationScheme,

Add support for dedicated allocations and committed resource tracking #148

Add support for dedicated allocations and committed resource tracking #148

Conversation

janie177 commented Dec 21, 2022 • edited Loading

Changes

Vulkan

D3D12

Testing

janie177 Dec 23, 2022

Choose a reason for hiding this comment

MarijnS95 Jan 12, 2023

Choose a reason for hiding this comment

MarijnS95 Jan 12, 2023

Choose a reason for hiding this comment

MarijnS95 Jan 12, 2023

Choose a reason for hiding this comment

MarijnS95 Jan 12, 2023

Choose a reason for hiding this comment

MarijnS95 Jan 12, 2023

Choose a reason for hiding this comment

MarijnS95 Jan 12, 2023

Choose a reason for hiding this comment

janie177 commented Dec 21, 2022 •

edited

Loading