Always keep buffers in vulkan persistently mapped.

This should hopefully fix (and possibly even improve) some of the perf
regressions we had form switching the memory allocator which were caused
by us now mapping and unmapping the entire VkDeviceMemory instead of just
subsets.

Bug: skia:
Change-Id: Ia8232594f33003e88969bf16febea4e4eec0ac95
Reviewed-on: https://skia-review.googlesource.com/131443
Commit-Queue: Greg Daniel <egdaniel@google.com>
Reviewed-by: Jim Van Verth <jvanverth@google.com>

3 files changed, 20 insertions, 4 deletions

diff --git a/include/gpu/vk/GrVkMemoryAllocator.h b/include/gpu/vk/GrVkMemoryAllocator.h
index 0e73586c3b..aec8200c9d 100644
--- a/include/gpu/vk/GrVkMemoryAllocator.h
+++ b/include/gpu/vk/GrVkMemoryAllocator.h
@@ -28,7 +28,7 @@ public:
         // The allocation will be mapped immediately and stay mapped until it is destroyed. This
         // flag is only valid for buffers which are host visible (i.e. must have a usage other than
         // BufferUsage::kGpuOnly).
-        kPersistentlyMapped  = 0x3,
+        kPersistentlyMapped  = 0x4,
     };
 
     GR_DECL_BITFIELD_CLASS_OPS_FRIENDS(AllocationPropertyFlags);
diff --git a/src/gpu/vk/GrVkAMDMemoryAllocator.cpp b/src/gpu/vk/GrVkAMDMemoryAllocator.cpp
index 53703a2149..21a35bc0e5 100644
--- a/src/gpu/vk/GrVkAMDMemoryAllocator.cpp
+++ b/src/gpu/vk/GrVkAMDMemoryAllocator.cpp
@@ -130,7 +130,8 @@ bool GrVkAMDMemoryAllocator::allocateMemoryForBuffer(VkBuffer buffer, BufferUsag
         info.preferredFlags |= VK_MEMORY_PROPERTY_LAZILY_ALLOCATED_BIT;
     }
 
-    if ((AllocationPropertyFlags::kPersistentlyMapped & flags) && BufferUsage::kGpuOnly != usage) {
+    if (AllocationPropertyFlags::kPersistentlyMapped & flags) {
+        SkASSERT(BufferUsage::kGpuOnly != usage);
         info.flags |= VMA_ALLOCATION_CREATE_MAPPED_BIT;
     }
 
diff --git a/src/gpu/vk/GrVkMemory.cpp b/src/gpu/vk/GrVkMemory.cpp
index f999c26546..942fc3c603 100644
--- a/src/gpu/vk/GrVkMemory.cpp
+++ b/src/gpu/vk/GrVkMemory.cpp
@@ -41,8 +41,23 @@ bool GrVkMemory::AllocAndBindBufferMemory(const GrVkGpu* gpu,
 
     GrVkMemoryAllocator::BufferUsage usage = get_buffer_usage(type, dynamic);
 
-    if (!allocator->allocateMemoryForBuffer(buffer, usage, AllocationPropertyFlags::kNone,
-                                            &memory)) {
+    AllocationPropertyFlags propFlags;
+    if (usage == GrVkMemoryAllocator::BufferUsage::kCpuWritesGpuReads) {
+        // In general it is always fine (and often better) to keep buffers always mapped.
+        // TODO: According to AMDs guide for the VulkanMemoryAllocator they suggest there are two
+        // cases when keeping it mapped can hurt. The first is when running on Win7 or Win8 (Win 10
+        // is fine). In general, by the time Vulkan ships it is probably less likely to be running
+        // on non Win10 or newer machines. The second use case is if running on an AMD card and you
+        // are using the special GPU local and host mappable memory. However, in general we don't
+        // pick this memory as we've found it slower than using the cached host visible memory. In
+        // the future if we find the need to special case either of these two issues we can add
+        // checks for them here.
+        propFlags = AllocationPropertyFlags::kPersistentlyMapped;
+    } else {
+        propFlags = AllocationPropertyFlags::kNone;
+    }
+
+    if (!allocator->allocateMemoryForBuffer(buffer, usage, propFlags, &memory)) {
         return false;
     }
     allocator->getAllocInfo(memory, alloc);