Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

VulkanDeviceAlloc: Memorytype per slab #11744

Merged
merged 2 commits into from
Jan 23, 2019
Merged

VulkanDeviceAlloc: Memorytype per slab #11744

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c8e5dd3
VK device allocator: Support separate memory type per slab.
hrydgard Jan 23, 2019
46585a5
Additional logging
hrydgard Jan 23, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Additional logging
  • Loading branch information
@hrydgard
hrydgard committed Jan 23, 2019
commit 46585a5da9a276499e78afdcf5d6ced311eef86d
8 changes: 8 additions & 0 deletions Common/Vulkan/VulkanContext.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -507,6 +507,14 @@ void VulkanContext::ChooseDevice(int physical_device) {

// This is as good a place as any to do this
vkGetPhysicalDeviceMemoryProperties(physical_devices_[physical_device_], &memory_properties);
ILOG("Memory Types (%d):", memory_properties.memoryTypeCount);
for (int i = 0; i < memory_properties.memoryTypeCount; i++) {
ILOG(" %d: Heap %d; Flags: %s%s%s%s ", i, memory_properties.memoryTypes[i].heapIndex,
(memory_properties.memoryTypes[i].propertyFlags & VK_MEMORY_PROPERTY_DEVICE_LOCAL_BIT) ? "DEVICE_LOCAL_BIT" : "",
(memory_properties.memoryTypes[i].propertyFlags & VK_MEMORY_PROPERTY_HOST_VISIBLE_BIT) ? "HOST_VISIBLE_BIT" : "",
(memory_properties.memoryTypes[i].propertyFlags & VK_MEMORY_PROPERTY_HOST_CACHED_BIT) ? "HOST_CACHED_BIT" : "",
(memory_properties.memoryTypes[i].propertyFlags & VK_MEMORY_PROPERTY_HOST_COHERENT_BIT) ? "HOST_COHERENT_BIT" : "");
}

// Optional features
vkGetPhysicalDeviceFeatures(physical_devices_[physical_device_], &featuresAvailable_);
Expand Down
6 changes: 6 additions & 0 deletions Common/Vulkan/VulkanImage.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ bool VulkanTexture::CreateDirect(VkCommandBuffer cmd, int w, int h, int numMips,
VkResult res = vkCreateImage(vulkan_->GetDevice(), &image_create_info, NULL, &image_);
if (res != VK_SUCCESS) {
_assert_(res == VK_ERROR_OUT_OF_HOST_MEMORY || res == VK_ERROR_OUT_OF_DEVICE_MEMORY || res == VK_ERROR_TOO_MANY_OBJECTS);
ELOG("vkCreateImage failed: %s", VulkanResultToString(res));
return false;
}

Expand All @@ -76,6 +77,8 @@ bool VulkanTexture::CreateDirect(VkCommandBuffer cmd, int w, int h, int numMips,
if (allocator_) {
offset_ = allocator_->Allocate(mem_reqs, &mem_, Tag());
if (offset_ == VulkanDeviceAllocator::ALLOCATE_FAILED) {
vkDestroyImage(vulkan_->GetDevice(), image_, nullptr);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Destroy is called in the destructor which already does this. We should at least clear image_, but I'm personally not a big fan of repeating WET cleanup code in every error case, so I try to design things so they auto-cleanup and the code is easier to read.

-[Unknown]

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

WET? Yeah I see your point, I'll clean it up.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We Enjoy Typing. The opposite of DRY.

-[Unknown]

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah :)

ELOG("Image memory allocation failed (mem_reqs.size = %d)", (int)mem_reqs.size);
return false;
}
} else {
Expand All @@ -89,6 +92,7 @@ bool VulkanTexture::CreateDirect(VkCommandBuffer cmd, int w, int h, int numMips,

res = vkAllocateMemory(vulkan_->GetDevice(), &mem_alloc, NULL, &mem_);
if (res != VK_SUCCESS) {
ELOG("vkAllocateMemory failed: %s", VulkanResultToString(res));
_assert_msg_(G3D, res != VK_ERROR_TOO_MANY_OBJECTS, "Too many Vulkan memory objects!");
_assert_(res == VK_ERROR_OUT_OF_HOST_MEMORY || res == VK_ERROR_OUT_OF_DEVICE_MEMORY || res == VK_ERROR_TOO_MANY_OBJECTS);
vkDestroyImage(vulkan_->GetDevice(), image_, nullptr);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FWIW, this one is also not necessary but at least clears image_ so is fine.

Expand All @@ -101,6 +105,7 @@ bool VulkanTexture::CreateDirect(VkCommandBuffer cmd, int w, int h, int numMips,

res = vkBindImageMemory(vulkan_->GetDevice(), image_, mem_, offset_);
if (res != VK_SUCCESS) {
ELOG("vkBindImageMemory failed: %s", VulkanResultToString(res));
// This leaks the image and memory. Should not really happen though...
_assert_(res == VK_ERROR_OUT_OF_HOST_MEMORY || res == VK_ERROR_OUT_OF_DEVICE_MEMORY || res == VK_ERROR_TOO_MANY_OBJECTS);
return false;
Expand Down Expand Up @@ -144,6 +149,7 @@ bool VulkanTexture::CreateDirect(VkCommandBuffer cmd, int w, int h, int numMips,

res = vkCreateImageView(vulkan_->GetDevice(), &view_info, NULL, &view_);
if (res != VK_SUCCESS) {
ELOG("vkCreateImageView failed: %s", VulkanResultToString(res));
// This leaks the image.
_assert_(res == VK_ERROR_OUT_OF_HOST_MEMORY || res == VK_ERROR_OUT_OF_DEVICE_MEMORY || res == VK_ERROR_TOO_MANY_OBJECTS);
return false;
Expand Down
2 changes: 1 addition & 1 deletion Common/Vulkan/VulkanMemory.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -191,8 +191,8 @@ size_t VulkanDeviceAllocator::Allocate(const VkMemoryRequirements &reqs, VkDevic
assert(!destroyed_);
uint32_t memoryTypeIndex;
bool pass = vulkan_->MemoryTypeFromProperties(reqs.memoryTypeBits, VK_MEMORY_PROPERTY_DEVICE_LOCAL_BIT, &memoryTypeIndex);
assert(pass);
if (!pass) {
ELOG("Failed to pick an appropriate memory type (req: %08x)", reqs.memoryTypeBits);
return ALLOCATE_FAILED;
}

Expand Down
1 change: 1 addition & 0 deletions android/jni/AndroidVulkanContext.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,7 @@ bool AndroidVulkanContext::InitAPI() {
g_Vulkan = nullptr;
return false;
}
ILOG("Vulkan device created!");
return true;
}

Expand Down