Memory manager: buffer uploads (Vulkan) - Part 2
Manages a single persistently mapped GPU heap which is sub-allocated
inside of ring-buffer for uploads. To handle larger buffers without additional
unused heaps, ring buffers are created on-demand.
BUG=dawn:28
TEST=dawn_unittests
Change-Id: Ic2a5df3142fc24fa772b9a85b38248eea8c7e003
Reviewed-on: https://dawn-review.googlesource.com/c/4260
Commit-Queue: Bryan Bernhart <bryan.bernhart@intel.com>
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
diff --git a/BUILD.gn b/BUILD.gn
index b5f4146..60bda32 100644
--- a/BUILD.gn
+++ b/BUILD.gn
@@ -659,8 +659,6 @@
"src/dawn_native/vulkan/BindGroupLayoutVk.h",
"src/dawn_native/vulkan/BindGroupVk.cpp",
"src/dawn_native/vulkan/BindGroupVk.h",
- "src/dawn_native/vulkan/BufferUploader.cpp",
- "src/dawn_native/vulkan/BufferUploader.h",
"src/dawn_native/vulkan/BufferVk.cpp",
"src/dawn_native/vulkan/BufferVk.h",
"src/dawn_native/vulkan/CommandBufferVk.cpp",
@@ -692,6 +690,8 @@
"src/dawn_native/vulkan/SamplerVk.h",
"src/dawn_native/vulkan/ShaderModuleVk.cpp",
"src/dawn_native/vulkan/ShaderModuleVk.h",
+ "src/dawn_native/vulkan/StagingBufferVk.cpp",
+ "src/dawn_native/vulkan/StagingBufferVk.h",
"src/dawn_native/vulkan/SwapChainVk.cpp",
"src/dawn_native/vulkan/SwapChainVk.h",
"src/dawn_native/vulkan/TextureVk.cpp",
diff --git a/src/dawn_native/vulkan/BufferUploader.cpp b/src/dawn_native/vulkan/BufferUploader.cpp
deleted file mode 100644
index e74143f..0000000
--- a/src/dawn_native/vulkan/BufferUploader.cpp
+++ /dev/null
@@ -1,102 +0,0 @@
-// Copyright 2017 The Dawn Authors
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-// http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-
-#include "dawn_native/vulkan/BufferUploader.h"
-
-#include "dawn_native/vulkan/DeviceVk.h"
-#include "dawn_native/vulkan/FencedDeleter.h"
-#include "dawn_native/vulkan/MemoryAllocator.h"
-
-#include <cstring>
-
-namespace dawn_native { namespace vulkan {
-
- BufferUploader::BufferUploader(Device* device) : mDevice(device) {
- }
-
- BufferUploader::~BufferUploader() {
- }
-
- void BufferUploader::BufferSubData(VkBuffer buffer,
- VkDeviceSize offset,
- VkDeviceSize size,
- const void* data) {
- // TODO(cwallez@chromium.org): this is soooooo bad. We should use some sort of ring buffer
- // for this.
-
- // Create a staging buffer
- VkBufferCreateInfo createInfo;
- createInfo.sType = VK_STRUCTURE_TYPE_BUFFER_CREATE_INFO;
- createInfo.pNext = nullptr;
- createInfo.flags = 0;
- createInfo.size = size;
- createInfo.usage = VK_BUFFER_USAGE_TRANSFER_SRC_BIT;
- createInfo.sharingMode = VK_SHARING_MODE_EXCLUSIVE;
- createInfo.queueFamilyIndexCount = 0;
- createInfo.pQueueFamilyIndices = 0;
-
- VkBuffer stagingBuffer = VK_NULL_HANDLE;
- if (mDevice->fn.CreateBuffer(mDevice->GetVkDevice(), &createInfo, nullptr,
- &stagingBuffer) != VK_SUCCESS) {
- ASSERT(false);
- }
-
- VkMemoryRequirements requirements;
- mDevice->fn.GetBufferMemoryRequirements(mDevice->GetVkDevice(), stagingBuffer,
- &requirements);
-
- DeviceMemoryAllocation allocation;
- if (!mDevice->GetMemoryAllocator()->Allocate(requirements, true, &allocation)) {
- ASSERT(false);
- }
-
- if (mDevice->fn.BindBufferMemory(mDevice->GetVkDevice(), stagingBuffer,
- allocation.GetMemory(),
- allocation.GetMemoryOffset()) != VK_SUCCESS) {
- ASSERT(false);
- }
-
- // Write to the staging buffer
- ASSERT(allocation.GetMappedPointer() != nullptr);
- memcpy(allocation.GetMappedPointer(), data, static_cast<size_t>(size));
-
- // Enqueue host write -> transfer src barrier and copy command
- VkCommandBuffer commands = mDevice->GetPendingCommandBuffer();
-
- VkMemoryBarrier barrier;
- barrier.sType = VK_STRUCTURE_TYPE_MEMORY_BARRIER;
- barrier.pNext = nullptr;
- barrier.srcAccessMask = VK_ACCESS_HOST_WRITE_BIT;
- barrier.dstAccessMask = VK_ACCESS_TRANSFER_READ_BIT;
-
- mDevice->fn.CmdPipelineBarrier(commands, VK_PIPELINE_STAGE_HOST_BIT,
- VK_PIPELINE_STAGE_TRANSFER_BIT, 0, 1, &barrier, 0, nullptr,
- 0, nullptr);
-
- VkBufferCopy copy;
- copy.srcOffset = 0;
- copy.dstOffset = offset;
- copy.size = size;
- mDevice->fn.CmdCopyBuffer(commands, stagingBuffer, buffer, 1, ©);
-
- // TODO(cwallez@chromium.org): Buffers must be deleted before the memory.
- // This happens to work for now, but is fragile.
- mDevice->GetMemoryAllocator()->Free(&allocation);
- mDevice->GetFencedDeleter()->DeleteWhenUnused(stagingBuffer);
- }
-
- void BufferUploader::Tick(Serial) {
- }
-
-}} // namespace dawn_native::vulkan
diff --git a/src/dawn_native/vulkan/BufferUploader.h b/src/dawn_native/vulkan/BufferUploader.h
deleted file mode 100644
index 37ff0d7..0000000
--- a/src/dawn_native/vulkan/BufferUploader.h
+++ /dev/null
@@ -1,43 +0,0 @@
-// Copyright 2017 The Dawn Authors
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-// http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-
-#ifndef DAWNNATIVE_VULKAN_BUFFERUPLOADER_H_
-#define DAWNNATIVE_VULKAN_BUFFERUPLOADER_H_
-
-#include "common/SerialQueue.h"
-#include "common/vulkan_platform.h"
-
-namespace dawn_native { namespace vulkan {
-
- class Device;
-
- class BufferUploader {
- public:
- BufferUploader(Device* device);
- ~BufferUploader();
-
- void BufferSubData(VkBuffer buffer,
- VkDeviceSize offset,
- VkDeviceSize size,
- const void* data);
-
- void Tick(Serial completedSerial);
-
- private:
- Device* mDevice = nullptr;
- };
-
-}} // namespace dawn_native::vulkan
-
-#endif // DAWNNATIVE_VULKAN_BUFFERUPLOADER_H_
diff --git a/src/dawn_native/vulkan/BufferVk.cpp b/src/dawn_native/vulkan/BufferVk.cpp
index 4175097..0080e76 100644
--- a/src/dawn_native/vulkan/BufferVk.cpp
+++ b/src/dawn_native/vulkan/BufferVk.cpp
@@ -14,7 +14,7 @@
#include "dawn_native/vulkan/BufferVk.h"
-#include "dawn_native/vulkan/BufferUploader.h"
+#include "dawn_native/DynamicUploader.h"
#include "dawn_native/vulkan/DeviceVk.h"
#include "dawn_native/vulkan/FencedDeleter.h"
@@ -199,11 +199,18 @@
MaybeError Buffer::SetSubDataImpl(uint32_t start, uint32_t count, const uint8_t* data) {
Device* device = ToBackend(GetDevice());
- VkCommandBuffer commands = device->GetPendingCommandBuffer();
- TransitionUsageNow(commands, dawn::BufferUsageBit::TransferDst);
+ DynamicUploader* uploader = nullptr;
+ DAWN_TRY_ASSIGN(uploader, device->GetDynamicUploader());
- BufferUploader* uploader = device->GetBufferUploader();
- uploader->BufferSubData(mHandle, start, count, data);
+ UploadHandle uploadHandle;
+ DAWN_TRY_ASSIGN(uploadHandle, uploader->Allocate(count, kDefaultAlignment));
+ ASSERT(uploadHandle.mappedBuffer != nullptr);
+
+ memcpy(uploadHandle.mappedBuffer, data, count);
+
+ DAWN_TRY(device->CopyFromStagingToBuffer(uploadHandle.stagingBuffer,
+ uploadHandle.startOffset, this, start, count));
+
return {};
}
diff --git a/src/dawn_native/vulkan/BufferVk.h b/src/dawn_native/vulkan/BufferVk.h
index ae90b4f..70636fc 100644
--- a/src/dawn_native/vulkan/BufferVk.h
+++ b/src/dawn_native/vulkan/BufferVk.h
@@ -46,6 +46,9 @@
void MapWriteAsyncImpl(uint32_t serial, uint32_t start, uint32_t count) override;
void UnmapImpl() override;
+ // TODO(b-brber): Remove once alignment constraint is added to validation (dawn:73).
+ static constexpr size_t kDefaultAlignment = 4; // TODO(b-brber): Figure out this value.
+
VkBuffer mHandle = VK_NULL_HANDLE;
DeviceMemoryAllocation mMemoryAllocation;
diff --git a/src/dawn_native/vulkan/DeviceVk.cpp b/src/dawn_native/vulkan/DeviceVk.cpp
index 32873c3..8fcd392 100644
--- a/src/dawn_native/vulkan/DeviceVk.cpp
+++ b/src/dawn_native/vulkan/DeviceVk.cpp
@@ -23,7 +23,6 @@
#include "dawn_native/vulkan/BackendVk.h"
#include "dawn_native/vulkan/BindGroupLayoutVk.h"
#include "dawn_native/vulkan/BindGroupVk.h"
-#include "dawn_native/vulkan/BufferUploader.h"
#include "dawn_native/vulkan/BufferVk.h"
#include "dawn_native/vulkan/CommandBufferVk.h"
#include "dawn_native/vulkan/ComputePipelineVk.h"
@@ -36,6 +35,7 @@
#include "dawn_native/vulkan/RenderPipelineVk.h"
#include "dawn_native/vulkan/SamplerVk.h"
#include "dawn_native/vulkan/ShaderModuleVk.h"
+#include "dawn_native/vulkan/StagingBufferVk.h"
#include "dawn_native/vulkan/SwapChainVk.h"
#include "dawn_native/vulkan/TextureVk.h"
#include "dawn_native/vulkan/VulkanError.h"
@@ -61,11 +61,11 @@
DAWN_TRY(functions->LoadDeviceProcs(mVkDevice, mDeviceInfo));
GatherQueueFromDevice();
-
- mBufferUploader = std::make_unique<BufferUploader>(this);
mDeleter = std::make_unique<FencedDeleter>(this);
mMapRequestTracker = std::make_unique<MapRequestTracker>(this);
mMemoryAllocator = std::make_unique<MemoryAllocator>(this);
+ mDynamicUploader = std::make_unique<DynamicUploader>(this);
+
mRenderPassCache = std::make_unique<RenderPassCache>(this);
return {};
@@ -116,7 +116,12 @@
mUnusedFences.clear();
// Free services explicitly so that they can free Vulkan objects before vkDestroyDevice
- mBufferUploader = nullptr;
+ mDynamicUploader = nullptr;
+
+ // Releasing the uploader enqueues buffers to be deleted.
+ // Call Tick() again to allow the deleter to clear them prior to being released.
+ mDeleter->Tick(mCompletedSerial);
+
mDeleter = nullptr;
mMapRequestTracker = nullptr;
mMemoryAllocator = nullptr;
@@ -205,7 +210,7 @@
RecycleCompletedCommands();
mMapRequestTracker->Tick(mCompletedSerial);
- mBufferUploader->Tick(mCompletedSerial);
+ mDynamicUploader->Tick(mCompletedSerial);
mMemoryAllocator->Tick(mCompletedSerial);
mDeleter->Tick(mCompletedSerial);
@@ -247,10 +252,6 @@
return mMemoryAllocator.get();
}
- BufferUploader* Device::GetBufferUploader() const {
- return mBufferUploader.get();
- }
-
FencedDeleter* Device::GetFencedDeleter() const {
return mDeleter.get();
}
@@ -497,7 +498,9 @@
}
ResultOrError<std::unique_ptr<StagingBufferBase>> Device::CreateStagingBuffer(size_t size) {
- return DAWN_UNIMPLEMENTED_ERROR("Device unable to create staging buffer.");
+ std::unique_ptr<StagingBufferBase> stagingBuffer =
+ std::make_unique<StagingBuffer>(size, this);
+ return std::move(stagingBuffer);
}
MaybeError Device::CopyFromStagingToBuffer(StagingBufferBase* source,
@@ -505,7 +508,35 @@
BufferBase* destination,
uint32_t destinationOffset,
uint32_t size) {
- return DAWN_UNIMPLEMENTED_ERROR("Device unable to copy from staging buffer.");
+ // Insert memory barrier to ensure host write operations are made visible before
+ // copying from the staging buffer. However, this barrier can be removed (see note below).
+ //
+ // Note: Depending on the spec understanding, an explicit barrier may not be required when
+ // used with HOST_COHERENT as vkQueueSubmit does an implicit barrier between host and
+ // device. See "Availability, Visibility, and Domain Operations" in Vulkan spec for details.
+
+ // Insert pipeline barrier to ensure correct ordering with previous memory operations on the
+ // buffer.
+ ToBackend(destination)
+ ->TransitionUsageNow(GetPendingCommandBuffer(), dawn::BufferUsageBit::TransferDst);
+
+ VkBufferCopy copy;
+ copy.srcOffset = sourceOffset;
+ copy.dstOffset = destinationOffset;
+ copy.size = size;
+
+ this->fn.CmdCopyBuffer(GetPendingCommandBuffer(), ToBackend(source)->GetBufferHandle(),
+ ToBackend(destination)->GetHandle(), 1, ©);
+
+ return {};
+ }
+
+ ResultOrError<DynamicUploader*> Device::GetDynamicUploader() const {
+ // TODO(b-brber): Refactor this into device init once moved into DeviceBase.
+ if (mDynamicUploader->IsEmpty()) {
+ DAWN_TRY(mDynamicUploader->CreateAndAppendBuffer(kDefaultUploadBufferSize));
+ }
+ return mDynamicUploader.get();
}
}} // namespace dawn_native::vulkan
diff --git a/src/dawn_native/vulkan/DeviceVk.h b/src/dawn_native/vulkan/DeviceVk.h
index aefd00d..ffa2a0d 100644
--- a/src/dawn_native/vulkan/DeviceVk.h
+++ b/src/dawn_native/vulkan/DeviceVk.h
@@ -81,6 +81,8 @@
uint32_t destinationOffset,
uint32_t size) override;
+ ResultOrError<DynamicUploader*> GetDynamicUploader() const;
+
private:
ResultOrError<BindGroupBase*> CreateBindGroupImpl(
const BindGroupDescriptor* descriptor) override;
@@ -114,7 +116,7 @@
uint32_t mQueueFamily = 0;
VkQueue mQueue = VK_NULL_HANDLE;
- std::unique_ptr<BufferUploader> mBufferUploader;
+ std::unique_ptr<DynamicUploader> mDynamicUploader;
std::unique_ptr<FencedDeleter> mDeleter;
std::unique_ptr<MapRequestTracker> mMapRequestTracker;
std::unique_ptr<MemoryAllocator> mMemoryAllocator;
@@ -145,6 +147,9 @@
std::vector<CommandPoolAndBuffer> mUnusedCommands;
CommandPoolAndBuffer mPendingCommands;
std::vector<VkSemaphore> mWaitSemaphores;
+
+ static constexpr size_t kDefaultUploadBufferSize =
+ 64000; // TODO(b-brber): Figure out this value.
};
}} // namespace dawn_native::vulkan
diff --git a/src/dawn_native/vulkan/Forward.h b/src/dawn_native/vulkan/Forward.h
index aade94e..5e6cac6 100644
--- a/src/dawn_native/vulkan/Forward.h
+++ b/src/dawn_native/vulkan/Forward.h
@@ -33,6 +33,7 @@
class RenderPipeline;
class Sampler;
class ShaderModule;
+ class StagingBuffer;
class SwapChain;
class Texture;
class TextureView;
@@ -52,6 +53,7 @@
using RenderPipelineType = RenderPipeline;
using SamplerType = Sampler;
using ShaderModuleType = ShaderModule;
+ using StagingBufferType = StagingBuffer;
using SwapChainType = SwapChain;
using TextureType = Texture;
using TextureViewType = TextureView;
diff --git a/src/dawn_native/vulkan/MemoryAllocator.cpp b/src/dawn_native/vulkan/MemoryAllocator.cpp
index e71adcb..6d7cf07 100644
--- a/src/dawn_native/vulkan/MemoryAllocator.cpp
+++ b/src/dawn_native/vulkan/MemoryAllocator.cpp
@@ -60,6 +60,12 @@
continue;
}
+ // Mappable must also be host coherent.
+ if (mappable &&
+ (info.memoryTypes[i].propertyFlags & VK_MEMORY_PROPERTY_HOST_COHERENT_BIT) == 0) {
+ continue;
+ }
+
// Found the first candidate memory type
if (bestType == -1) {
bestType = static_cast<int>(i);
diff --git a/src/dawn_native/vulkan/StagingBufferVk.cpp b/src/dawn_native/vulkan/StagingBufferVk.cpp
new file mode 100644
index 0000000..4e96f85
--- /dev/null
+++ b/src/dawn_native/vulkan/StagingBufferVk.cpp
@@ -0,0 +1,72 @@
+// Copyright 2018 The Dawn Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#include "dawn_native/vulkan/StagingBufferVk.h"
+#include "dawn_native/vulkan/DeviceVk.h"
+#include "dawn_native/vulkan/FencedDeleter.h"
+#include "dawn_native/vulkan/MemoryAllocator.h"
+
+namespace dawn_native { namespace vulkan {
+
+ StagingBuffer::StagingBuffer(size_t size, Device* device)
+ : StagingBufferBase(size), mDevice(device) {
+ }
+
+ MaybeError StagingBuffer::Initialize() {
+ VkBufferCreateInfo createInfo;
+ createInfo.sType = VK_STRUCTURE_TYPE_BUFFER_CREATE_INFO;
+ createInfo.pNext = nullptr;
+ createInfo.flags = 0;
+ createInfo.size = GetSize();
+ createInfo.usage = VK_BUFFER_USAGE_TRANSFER_SRC_BIT;
+ createInfo.sharingMode = VK_SHARING_MODE_EXCLUSIVE;
+ createInfo.queueFamilyIndexCount = 0;
+ createInfo.pQueueFamilyIndices = 0;
+
+ if (mDevice->fn.CreateBuffer(mDevice->GetVkDevice(), &createInfo, nullptr, &mBuffer) !=
+ VK_SUCCESS) {
+ return DAWN_CONTEXT_LOST_ERROR("Unable to create staging buffer.");
+ }
+
+ VkMemoryRequirements requirements;
+ mDevice->fn.GetBufferMemoryRequirements(mDevice->GetVkDevice(), mBuffer, &requirements);
+
+ if (!mDevice->GetMemoryAllocator()->Allocate(requirements, true, &mAllocation)) {
+ return DAWN_CONTEXT_LOST_ERROR("Unable to allocate memory for staging buffer.");
+ }
+
+ if (mDevice->fn.BindBufferMemory(mDevice->GetVkDevice(), mBuffer, mAllocation.GetMemory(),
+ mAllocation.GetMemoryOffset()) != VK_SUCCESS) {
+ return DAWN_CONTEXT_LOST_ERROR("Unable to attach memory to the staging buffer.");
+ }
+
+ mMappedPointer = mAllocation.GetMappedPointer();
+ if (mMappedPointer == nullptr) {
+ return DAWN_CONTEXT_LOST_ERROR("Unable to map staging buffer.");
+ }
+
+ return {};
+ }
+
+ StagingBuffer::~StagingBuffer() {
+ mMappedPointer = nullptr;
+ mDevice->GetFencedDeleter()->DeleteWhenUnused(mBuffer);
+ mDevice->GetMemoryAllocator()->Free(&mAllocation);
+ }
+
+ VkBuffer StagingBuffer::GetBufferHandle() const {
+ return mBuffer;
+ }
+
+}} // namespace dawn_native::vulkan
\ No newline at end of file
diff --git a/src/dawn_native/vulkan/StagingBufferVk.h b/src/dawn_native/vulkan/StagingBufferVk.h
new file mode 100644
index 0000000..618c5ed
--- /dev/null
+++ b/src/dawn_native/vulkan/StagingBufferVk.h
@@ -0,0 +1,41 @@
+// Copyright 2018 The Dawn Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#ifndef DAWNNATIVE_STAGINGBUFFERVK_H_
+#define DAWNNATIVE_STAGINGBUFFERVK_H_
+
+#include "dawn_native/StagingBuffer.h"
+#include "dawn_native/vulkan/MemoryAllocator.h"
+
+namespace dawn_native { namespace vulkan {
+
+ class Device;
+
+ class StagingBuffer : public StagingBufferBase {
+ public:
+ StagingBuffer(size_t size, Device* device);
+ ~StagingBuffer();
+
+ VkBuffer GetBufferHandle() const;
+
+ MaybeError Initialize() override;
+
+ private:
+ Device* mDevice;
+ VkBuffer mBuffer;
+ DeviceMemoryAllocation mAllocation;
+ };
+}} // namespace dawn_native::vulkan
+
+#endif // DAWNNATIVE_STAGINGBUFFERVK_H_
diff --git a/src/tests/end2end/BufferTests.cpp b/src/tests/end2end/BufferTests.cpp
index 70dfac8..399994f 100644
--- a/src/tests/end2end/BufferTests.cpp
+++ b/src/tests/end2end/BufferTests.cpp
@@ -222,7 +222,7 @@
TEST_P(BufferSetSubDataTests, ManySetSubData) {
// TODO(cwallez@chromium.org): Use ringbuffers for SetSubData on explicit APIs.
// otherwise this creates too many resources and can take freeze the driver(?)
- DAWN_SKIP_TEST_IF(IsMetal() || IsVulkan());
+ DAWN_SKIP_TEST_IF(IsMetal());
// Note: Increasing the size of the buffer will likely cause timeout issues.
// In D3D12, timeout detection occurs when the GPU scheduler tries but cannot preempt the task
