Memory manager: buffer uploads (Metal+refactor) - Part 3

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: If4d3e717186895b1409502c1dea5ab751a4776b2
Reviewed-on: https://dawn-review.googlesource.com/c/4440
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Reviewed-by: Kai Ninomiya <kainino@chromium.org>
Commit-Queue: Bryan Bernhart <bryan.bernhart@intel.com>

BUILD.gn

@@ -587,12 +587,12 @@ source_set("libdawn_native_sources") {
       "src/dawn_native/metal/QueueMTL.mm",
       "src/dawn_native/metal/RenderPipelineMTL.h",
       "src/dawn_native/metal/RenderPipelineMTL.mm",
-      "src/dawn_native/metal/ResourceUploader.h",
-      "src/dawn_native/metal/ResourceUploader.mm",
       "src/dawn_native/metal/SamplerMTL.h",
       "src/dawn_native/metal/SamplerMTL.mm",
       "src/dawn_native/metal/ShaderModuleMTL.h",
       "src/dawn_native/metal/ShaderModuleMTL.mm",
+      "src/dawn_native/metal/StagingBufferMTL.h",
+      "src/dawn_native/metal/StagingBufferMTL.mm",
       "src/dawn_native/metal/SwapChainMTL.h",
       "src/dawn_native/metal/SwapChainMTL.mm",
       "src/dawn_native/metal/TextureMTL.h",

src/dawn_native/Buffer.cpp

@@ -16,6 +16,7 @@
 
 #include "common/Assert.h"
 #include "dawn_native/Device.h"
+#include "dawn_native/DynamicUploader.h"
 #include "dawn_native/ValidationUtils_autogen.h"
 
 #include <cstdio>
@@ -183,6 +184,26 @@ namespace dawn_native {
         MapReadAsyncImpl(mMapSerial);
     }
 
+    MaybeError BufferBase::SetSubDataImpl(uint32_t start, uint32_t count, const uint8_t* data) {
+        DynamicUploader* uploader = nullptr;
+        DAWN_TRY_ASSIGN(uploader, GetDevice()->GetDynamicUploader());
+
+        // TODO(bryan.bernhart@intel.com): Remove once alignment constraint is added to validation
+        // (dawn:73). D3D12 does not specify so we assume 4-byte alignment to be safe.
+        static constexpr size_t kDefaultAlignment = 4;
+
+        UploadHandle uploadHandle;
+        DAWN_TRY_ASSIGN(uploadHandle, uploader->Allocate(count, kDefaultAlignment));
+        ASSERT(uploadHandle.mappedBuffer != nullptr);
+
+        memcpy(uploadHandle.mappedBuffer, data, count);
+
+        DAWN_TRY(GetDevice()->CopyFromStagingToBuffer(
+            uploadHandle.stagingBuffer, uploadHandle.startOffset, this, start, count));
+
+        return {};
+    }
+
     void BufferBase::MapWriteAsync(dawnBufferMapWriteCallback callback,
                                    dawnCallbackUserdata userdata) {
         if (GetDevice()->ConsumedError(ValidateMap(dawn::BufferUsageBit::MapWrite))) {

src/dawn_native/Buffer.h

@@ -72,7 +72,7 @@ namespace dawn_native {
                                   uint32_t dataLength);
 
       private:
-        virtual MaybeError SetSubDataImpl(uint32_t start, uint32_t count, const uint8_t* data) = 0;
+        virtual MaybeError SetSubDataImpl(uint32_t start, uint32_t count, const uint8_t* data);
         virtual void MapReadAsyncImpl(uint32_t serial) = 0;
         virtual void MapWriteAsyncImpl(uint32_t serial) = 0;
         virtual void UnmapImpl() = 0;

src/dawn_native/Device.cpp

@@ -21,6 +21,7 @@
 #include "dawn_native/CommandBuffer.h"
 #include "dawn_native/CommandEncoder.h"
 #include "dawn_native/ComputePipeline.h"
+#include "dawn_native/DynamicUploader.h"
 #include "dawn_native/ErrorData.h"
 #include "dawn_native/Fence.h"
 #include "dawn_native/FenceSignalTracker.h"
@@ -54,9 +55,12 @@ namespace dawn_native {
     DeviceBase::DeviceBase(AdapterBase* adapter) : mAdapter(adapter) {
         mCaches = std::make_unique<DeviceBase::Caches>();
         mFenceSignalTracker = std::make_unique<FenceSignalTracker>(this);
+        mDynamicUploader = std::make_unique<DynamicUploader>(this);
     }
 
     DeviceBase::~DeviceBase() {
+        // Devices must explicitly free the uploader
+        ASSERT(mDynamicUploader == nullptr);
     }
 
     void DeviceBase::HandleError(const char* message) {
@@ -377,4 +381,11 @@ namespace dawn_native {
         delete error;
     }
 
+    ResultOrError<DynamicUploader*> DeviceBase::GetDynamicUploader() const {
+        if (mDynamicUploader->IsEmpty()) {
+            DAWN_TRY(mDynamicUploader->CreateAndAppendBuffer());
+        }
+        return mDynamicUploader.get();
+    }
+
 }  // namespace dawn_native

src/dawn_native/Device.h

@@ -124,6 +124,11 @@ namespace dawn_native {
                                                    uint32_t destinationOffset,
                                                    uint32_t size) = 0;
 
+        ResultOrError<DynamicUploader*> GetDynamicUploader() const;
+
+      protected:
+        std::unique_ptr<DynamicUploader> mDynamicUploader;
+
       private:
         virtual ResultOrError<BindGroupBase*> CreateBindGroupImpl(
             const BindGroupDescriptor* descriptor) = 0;

src/dawn_native/DynamicUploader.cpp

@@ -46,7 +46,7 @@ namespace dawn_native {
                 newMaxSize *= 2;
             }
 
-            // TODO(b-brber): Fall-back to no sub-allocations should this fail.
+            // TODO(bryan.bernhart@intel.com): Fall-back to no sub-allocations should this fail.
             DAWN_TRY(CreateAndAppendBuffer(newMaxSize));
             largestRingBuffer = GetLargestBuffer();
             uploadHandle = largestRingBuffer->SubAllocate(alignedSize);

src/dawn_native/DynamicUploader.h

@@ -32,11 +32,14 @@ namespace dawn_native {
 
         RingBuffer* GetLargestBuffer();
 
-        MaybeError CreateAndAppendBuffer(size_t size);
+        MaybeError CreateAndAppendBuffer(size_t size = kBaseUploadBufferSize);
 
         bool IsEmpty() const;
 
       private:
+        // TODO(bryan.bernhart@intel.com): Figure out this value.
+        static constexpr size_t kBaseUploadBufferSize = 64000;
+
         std::vector<std::unique_ptr<RingBuffer>> mRingBuffers;
         DeviceBase* mDevice;
     };

src/dawn_native/RingBuffer.cpp

@@ -27,7 +27,7 @@
 // only two indices that keep increasing (unbounded) but can be still indexed using bit masks.
 // However, this 1) requires the size to always be a power-of-two and 2) remove tests that check
 // used bytes.
-// TODO(b-brber): Follow-up with ringbuffer optimization.
+// TODO(bryan.bernhart@intel.com): Follow-up with ringbuffer optimization.
 namespace dawn_native {
 
     static constexpr size_t INVALID_OFFSET = std::numeric_limits<size_t>::max();

src/dawn_native/d3d12/BufferD3D12.cpp

@@ -17,7 +17,6 @@
 #include "common/Assert.h"
 #include "common/Constants.h"
 #include "common/Math.h"
-#include "dawn_native/DynamicUploader.h"
 #include "dawn_native/d3d12/DeviceD3D12.h"
 #include "dawn_native/d3d12/ResourceAllocator.h"
 
@@ -161,24 +160,6 @@ namespace dawn_native { namespace d3d12 {
         }
     }
 
-    MaybeError Buffer::SetSubDataImpl(uint32_t start, uint32_t count, const uint8_t* data) {
-        Device* device = ToBackend(GetDevice());
-
-        DynamicUploader* uploader = nullptr;
-        DAWN_TRY_ASSIGN(uploader, device->GetDynamicUploader());
-
-        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 {};
-    }
-
     void Buffer::MapReadAsyncImpl(uint32_t serial) {
         mWrittenMappedRange = {};
         D3D12_RANGE readRange = {0, GetSize()};

src/dawn_native/d3d12/BufferD3D12.h

@@ -39,15 +39,10 @@ namespace dawn_native { namespace d3d12 {
 
       private:
         // Dawn API
-        MaybeError SetSubDataImpl(uint32_t start, uint32_t count, const uint8_t* data) override;
         void MapReadAsyncImpl(uint32_t serial) override;
         void MapWriteAsyncImpl(uint32_t serial) override;
         void UnmapImpl() override;
 
-        // TODO(b-brber): Remove once alignment constraint is added to validation (dawn:73).
-        static constexpr size_t kDefaultAlignment =
-            4;  // D3D does not specify so we assume 4-byte alignment to be safe.
-
         ComPtr<ID3D12Resource> mResource;
         bool mFixedResourceState = false;
         dawn::BufferUsageBit mLastUsage = dawn::BufferUsageBit::None;

src/dawn_native/d3d12/DeviceD3D12.cpp

@@ -63,7 +63,6 @@ namespace dawn_native { namespace d3d12 {
         mDescriptorHeapAllocator = std::make_unique<DescriptorHeapAllocator>(this);
         mMapRequestTracker = std::make_unique<MapRequestTracker>(this);
         mResourceAllocator = std::make_unique<ResourceAllocator>(this);
-        mDynamicUploader = std::make_unique<DynamicUploader>(this);
 
         NextSerial();
     }
@@ -73,6 +72,14 @@ namespace dawn_native { namespace d3d12 {
         WaitForSerial(mLastSubmittedSerial);  // Wait for all in-flight commands to finish executing
         TickImpl();                    // Call tick one last time so resources are cleaned up
 
+        // Free services explicitly so that they can free D3D12 resources before destruction of the
+        // device.
+        mDynamicUploader = nullptr;
+
+        // Releasing the uploader enqueues buffers to be released.
+        // Call Tick() again to clear them before releasing the allocator.
+        mResourceAllocator->Tick(mCompletedSerial);
+
         ASSERT(mUsedComObjectRefs.Empty());
         ASSERT(mPendingCommands.commandList == nullptr);
     }
@@ -143,12 +150,16 @@ namespace dawn_native { namespace d3d12 {
     void Device::TickImpl() {
         // Perform cleanup operations to free unused objects
         mCompletedSerial = mFence->GetCompletedValue();
+
+        // Uploader should tick before the resource allocator
+        // as it enqueues resources to be released.
+        mDynamicUploader->Tick(mCompletedSerial);
+
         mResourceAllocator->Tick(mCompletedSerial);
         mCommandAllocatorManager->Tick(mCompletedSerial);
         mDescriptorHeapAllocator->Tick(mCompletedSerial);
         mMapRequestTracker->Tick(mCompletedSerial);
         mUsedComObjectRefs.ClearUpTo(mCompletedSerial);
-        mDynamicUploader->Tick(mCompletedSerial);
         ExecuteCommandLists({});
         NextSerial();
     }
@@ -265,12 +276,4 @@ namespace dawn_native { namespace d3d12 {
         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::d3d12

src/dawn_native/d3d12/DeviceD3D12.h

@@ -77,8 +77,6 @@ namespace dawn_native { namespace d3d12 {
                                            uint32_t destinationOffset,
                                            uint32_t size) override;
 
-        ResultOrError<DynamicUploader*> GetDynamicUploader() const;
-
       private:
         ResultOrError<BindGroupBase*> CreateBindGroupImpl(
             const BindGroupDescriptor* descriptor) override;
@@ -121,11 +119,8 @@ namespace dawn_native { namespace d3d12 {
         std::unique_ptr<DescriptorHeapAllocator> mDescriptorHeapAllocator;
         std::unique_ptr<MapRequestTracker> mMapRequestTracker;
         std::unique_ptr<ResourceAllocator> mResourceAllocator;
-        std::unique_ptr<DynamicUploader> mDynamicUploader;
 
         dawn_native::PCIInfo mPCIInfo;
-
-        static constexpr size_t kDefaultUploadBufferSize = 64000;  // DXGI min heap size is 64kB.
     };
 
 }}  // namespace dawn_native::d3d12

src/dawn_native/d3d12/StagingBufferD3D12.cpp

@@ -39,7 +39,7 @@ namespace dawn_native { namespace d3d12 {
         mUploadHeap = mDevice->GetResourceAllocator()->Allocate(
             D3D12_HEAP_TYPE_UPLOAD, resourceDescriptor, D3D12_RESOURCE_STATE_GENERIC_READ);
 
-        // TODO(b-brber): Record the GPU pointer for generic non-upload usage.
+        // TODO(bryan.bernhart@intel.com): Record the GPU pointer for generic non-upload usage.
 
         if (FAILED(mUploadHeap->Map(0, nullptr, &mMappedPointer))) {
             return DAWN_CONTEXT_LOST_ERROR("Unable to map staging buffer.");

src/dawn_native/metal/BufferMTL.h

@@ -34,7 +34,6 @@ namespace dawn_native { namespace metal {
         void OnMapCommandSerialFinished(uint32_t mapSerial, bool isWrite);
 
       private:
-        MaybeError SetSubDataImpl(uint32_t start, uint32_t count, const uint8_t* data) override;
         void MapReadAsyncImpl(uint32_t serial) override;
         void MapWriteAsyncImpl(uint32_t serial) override;
         void UnmapImpl() override;

src/dawn_native/metal/BufferMTL.mm

@@ -15,7 +15,6 @@
 #include "dawn_native/metal/BufferMTL.h"
 
 #include "dawn_native/metal/DeviceMTL.h"
-#include "dawn_native/metal/ResourceUploader.h"
 
 namespace dawn_native { namespace metal {
 
@@ -49,12 +48,6 @@ namespace dawn_native { namespace metal {
         }
     }
 
-    MaybeError Buffer::SetSubDataImpl(uint32_t start, uint32_t count, const uint8_t* data) {
-        auto* uploader = ToBackend(GetDevice())->GetResourceUploader();
-        uploader->BufferSubData(mMtlBuffer, start, count, data);
-        return {};
-    }
-
     void Buffer::MapReadAsyncImpl(uint32_t serial) {
         MapRequestTracker* tracker = ToBackend(GetDevice())->GetMapTracker();
         tracker->Track(this, serial, false);

src/dawn_native/metal/DeviceMTL.h

@@ -30,7 +30,6 @@
 namespace dawn_native { namespace metal {
 
     class MapRequestTracker;
-    class ResourceUploader;
 
     class Device : public DeviceBase {
       public:
@@ -53,7 +52,6 @@ namespace dawn_native { namespace metal {
         void SubmitPendingCommandBuffer();
 
         MapRequestTracker* GetMapTracker() const;
-        ResourceUploader* GetResourceUploader() const;
 
         ResultOrError<std::unique_ptr<StagingBufferBase>> CreateStagingBuffer(size_t size) override;
         MaybeError CopyFromStagingToBuffer(StagingBufferBase* source,
@@ -90,7 +88,6 @@ namespace dawn_native { namespace metal {
         id<MTLDevice> mMtlDevice = nil;
         id<MTLCommandQueue> mCommandQueue = nil;
         std::unique_ptr<MapRequestTracker> mMapTracker;
-        std::unique_ptr<ResourceUploader> mResourceUploader;
 
         Serial mCompletedSerial = 0;
         Serial mLastSubmittedSerial = 0;

src/dawn_native/metal/DeviceMTL.mm

@@ -26,9 +26,9 @@
 #include "dawn_native/metal/PipelineLayoutMTL.h"
 #include "dawn_native/metal/QueueMTL.h"
 #include "dawn_native/metal/RenderPipelineMTL.h"
-#include "dawn_native/metal/ResourceUploader.h"
 #include "dawn_native/metal/SamplerMTL.h"
 #include "dawn_native/metal/ShaderModuleMTL.h"
+#include "dawn_native/metal/StagingBufferMTL.h"
 #include "dawn_native/metal/SwapChainMTL.h"
 #include "dawn_native/metal/TextureMTL.h"
 
@@ -37,8 +37,7 @@ namespace dawn_native { namespace metal {
     Device::Device(AdapterBase* adapter, id<MTLDevice> mtlDevice)
         : DeviceBase(adapter),
           mMtlDevice([mtlDevice retain]),
-          mMapTracker(new MapRequestTracker(this)),
+          mMapTracker(new MapRequestTracker(this)) {
-          mResourceUploader(new ResourceUploader(this)) {
         [mMtlDevice retain];
         mCommandQueue = [mMtlDevice newCommandQueue];
     }
@@ -58,7 +57,7 @@ namespace dawn_native { namespace metal {
         mPendingCommands = nil;
 
         mMapTracker = nullptr;
-        mResourceUploader = nullptr;
+        mDynamicUploader = nullptr;
 
         [mCommandQueue release];
         mCommandQueue = nil;
@@ -136,7 +135,7 @@ namespace dawn_native { namespace metal {
     }
 
     void Device::TickImpl() {
-        mResourceUploader->Tick(mCompletedSerial);
+        mDynamicUploader->Tick(mCompletedSerial);
         mMapTracker->Tick(mCompletedSerial);
 
         if (mPendingCommands != nil) {
@@ -185,12 +184,10 @@ namespace dawn_native { namespace metal {
         return mMapTracker.get();
     }
 
-    ResourceUploader* Device::GetResourceUploader() const {
-        return mResourceUploader.get();
-    }
-
     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,
@@ -198,7 +195,18 @@ namespace dawn_native { namespace metal {
                                                BufferBase* destination,
                                                uint32_t destinationOffset,
                                                uint32_t size) {
-        return DAWN_UNIMPLEMENTED_ERROR("Device unable to copy from staging buffer.");
+        id<MTLBuffer> uploadBuffer = ToBackend(source)->GetBufferHandle();
+        id<MTLBuffer> buffer = ToBackend(destination)->GetMTLBuffer();
+        id<MTLCommandBuffer> commandBuffer = GetPendingCommandBuffer();
+        id<MTLBlitCommandEncoder> encoder = [commandBuffer blitCommandEncoder];
+        [encoder copyFromBuffer:uploadBuffer
+                   sourceOffset:sourceOffset
+                       toBuffer:buffer
+              destinationOffset:destinationOffset
+                           size:size];
+        [encoder endEncoding];
+
+        return {};
     }
 
 }}  // namespace dawn_native::metal

src/dawn_native/metal/Forward.h

@@ -40,6 +40,7 @@ namespace dawn_native { namespace metal {
     class RenderPipeline;
     class Sampler;
     class ShaderModule;
+    class StagingBuffer;
     class SwapChain;
     class Texture;
     class TextureView;
@@ -59,6 +60,7 @@ namespace dawn_native { namespace metal {
         using RenderPipelineType = RenderPipeline;
         using SamplerType = Sampler;
         using ShaderModuleType = ShaderModule;
+        using StagingBufferType = StagingBuffer;
         using SwapChainType = SwapChain;
         using TextureType = Texture;
         using TextureViewType = TextureView;

src/dawn_native/metal/ResourceUploader.mm

@@ -1,57 +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/metal/ResourceUploader.h"
-
-#include "dawn_native/metal/DeviceMTL.h"
-
-namespace dawn_native { namespace metal {
-
-    ResourceUploader::ResourceUploader(Device* device) : mDevice(device) {
-    }
-
-    ResourceUploader::~ResourceUploader() {
-        ASSERT(mInflightUploadBuffers.Empty());
-    }
-
-    void ResourceUploader::BufferSubData(id<MTLBuffer> buffer,
-                                         uint32_t start,
-                                         uint32_t size,
-                                         const void* data) {
-        // TODO(cwallez@chromium.org) use a ringbuffer instead of creating a small buffer for each
-        // update
-        id<MTLBuffer> uploadBuffer =
-            [mDevice->GetMTLDevice() newBufferWithLength:size options:MTLResourceStorageModeShared];
-        memcpy([uploadBuffer contents], data, size);
-
-        id<MTLCommandBuffer> commandBuffer = mDevice->GetPendingCommandBuffer();
-        id<MTLBlitCommandEncoder> encoder = [commandBuffer blitCommandEncoder];
-        [encoder copyFromBuffer:uploadBuffer
-                   sourceOffset:0
-                       toBuffer:buffer
-              destinationOffset:start
-                           size:size];
-        [encoder endEncoding];
-
-        mInflightUploadBuffers.Enqueue(uploadBuffer, mDevice->GetPendingCommandSerial());
-    }
-
-    void ResourceUploader::Tick(Serial finishedSerial) {
-        for (id<MTLBuffer> buffer : mInflightUploadBuffers.IterateUpTo(finishedSerial)) {
-            [buffer release];
-        }
-        mInflightUploadBuffers.ClearUpTo(finishedSerial);
-    }
-
-}}  // namespace dawn_native::metal

src/dawn_native/metal/StagingBufferMTL.h

@@ -1,4 +1,4 @@
-// Copyright 2017 The Dawn Authors
+// 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.
@@ -12,11 +12,10 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-#ifndef DAWNNATIVE_METAL_RESOURCEUPLOADER_H_
+#ifndef DAWNNATIVE_STAGINGBUFFERMETAL_H_
-#define DAWNNATIVE_METAL_RESOURCEUPLOADER_H_
+#define DAWNNATIVE_STAGINGBUFFERMETAL_H_
 
-#include "common/Serial.h"
+#include "dawn_native/StagingBuffer.h"
-#include "common/SerialQueue.h"
 
 #import <Metal/Metal.h>
 
@@ -24,19 +23,19 @@ namespace dawn_native { namespace metal {
 
     class Device;
 
-    class ResourceUploader {
+    class StagingBuffer : public StagingBufferBase {
       public:
-        ResourceUploader(Device* device);
+        StagingBuffer(size_t size, Device* device);
-        ~ResourceUploader();
+        ~StagingBuffer();
 
-        void BufferSubData(id<MTLBuffer> buffer, uint32_t start, uint32_t size, const void* data);
+        id<MTLBuffer> GetBufferHandle() const;
-        void Tick(Serial finishedSerial);
+
+        MaybeError Initialize() override;
 
       private:
         Device* mDevice;
-        SerialQueue<id<MTLBuffer>> mInflightUploadBuffers;
+        id<MTLBuffer> mBuffer = nil;
     };
-
 }}  // namespace dawn_native::metal
 
-#endif  // DAWNNATIVE_METAL_RESOURCEUPLOADER_H_
+#endif  // DAWNNATIVE_STAGINGBUFFERMETAL_H_

src/dawn_native/metal/StagingBufferMTL.mm

@@ -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.
+
+#include "dawn_native/metal/StagingBufferMTL.h"
+#include "dawn_native/metal/DeviceMTL.h"
+
+namespace dawn_native { namespace metal {
+
+    StagingBuffer::StagingBuffer(size_t size, Device* device)
+        : StagingBufferBase(size), mDevice(device) {
+    }
+
+    MaybeError StagingBuffer::Initialize() {
+        const size_t bufferSize = GetSize();
+        mBuffer = [mDevice->GetMTLDevice() newBufferWithLength:bufferSize
+                                                       options:MTLResourceStorageModeShared];
+        mMappedPointer = [mBuffer contents];
+        return {};
+    }
+
+    StagingBuffer::~StagingBuffer() {
+        [mBuffer release];
+        mBuffer = nil;
+    }
+
+    id<MTLBuffer> StagingBuffer::GetBufferHandle() const {
+        return mBuffer;
+    }
+
+}}  // namespace dawn_native::metal

src/dawn_native/null/DeviceNull.cpp

@@ -58,10 +58,10 @@ namespace dawn_native { namespace null {
     // Device
 
     Device::Device(Adapter* adapter) : DeviceBase(adapter) {
-        mDynamicUploader = std::make_unique<DynamicUploader>(this);
     }
 
     Device::~Device() {
+        mDynamicUploader = nullptr;
     }
 
     ResultOrError<BindGroupBase*> Device::CreateBindGroupImpl(

src/dawn_native/null/DeviceNull.h

@@ -136,7 +136,6 @@ namespace dawn_native { namespace null {
         Serial mCompletedSerial = 0;
         Serial mLastSubmittedSerial = 0;
         std::vector<std::unique_ptr<PendingOperation>> mPendingOperations;
-        std::unique_ptr<DynamicUploader> mDynamicUploader;
     };
 
     class Buffer : public BufferBase {

src/dawn_native/opengl/DeviceGL.cpp

@@ -44,6 +44,9 @@ namespace dawn_native { namespace opengl {
         // on a serial that doesn't have a corresponding fence enqueued. Force all
         // operations to look as if they were completed (because they were).
         mCompletedSerial = mLastSubmittedSerial + 1;
+
+        mDynamicUploader = nullptr;
+
         Tick();
     }
 

src/dawn_native/vulkan/BufferVk.cpp

@@ -14,7 +14,6 @@
 
 #include "dawn_native/vulkan/BufferVk.h"
 
-#include "dawn_native/DynamicUploader.h"
 #include "dawn_native/vulkan/DeviceVk.h"
 #include "dawn_native/vulkan/FencedDeleter.h"
 
@@ -196,24 +195,6 @@ namespace dawn_native { namespace vulkan {
         mLastUsage = usage;
     }
 
-    MaybeError Buffer::SetSubDataImpl(uint32_t start, uint32_t count, const uint8_t* data) {
-        Device* device = ToBackend(GetDevice());
-
-        DynamicUploader* uploader = nullptr;
-        DAWN_TRY_ASSIGN(uploader, device->GetDynamicUploader());
-
-        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 {};
-    }
-
     void Buffer::MapReadAsyncImpl(uint32_t serial) {
         Device* device = ToBackend(GetDevice());
 

src/dawn_native/vulkan/BufferVk.h

@@ -41,14 +41,10 @@ namespace dawn_native { namespace vulkan {
         void TransitionUsageNow(VkCommandBuffer commands, dawn::BufferUsageBit usage);
 
       private:
-        MaybeError SetSubDataImpl(uint32_t start, uint32_t count, const uint8_t* data) override;
         void MapReadAsyncImpl(uint32_t serial) override;
         void MapWriteAsyncImpl(uint32_t serial) 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;
 

src/dawn_native/vulkan/DeviceVk.cpp

@@ -63,8 +63,6 @@ namespace dawn_native { namespace vulkan {
         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 {};
@@ -117,8 +115,8 @@ namespace dawn_native { namespace vulkan {
         // Free services explicitly so that they can free Vulkan objects before vkDestroyDevice
         mDynamicUploader = nullptr;
 
-        // Releasing the uploader enqueues buffers to be deleted.
+        // Releasing the uploader enqueues buffers to be released.
-        // Call Tick() again to allow the deleter to clear them prior to being released.
+        // Call Tick() again to clear them before releasing the deleter.
         mDeleter->Tick(mCompletedSerial);
 
         mDeleter = nullptr;
@@ -209,7 +207,11 @@ namespace dawn_native { namespace vulkan {
         RecycleCompletedCommands();
 
         mMapRequestTracker->Tick(mCompletedSerial);
+
+        // Uploader should tick before the resource allocator
+        // as it enqueues resources to be released.
         mDynamicUploader->Tick(mCompletedSerial);
+
         mMemoryAllocator->Tick(mCompletedSerial);
 
         mDeleter->Tick(mCompletedSerial);
@@ -529,13 +531,4 @@ namespace dawn_native { namespace vulkan {
 
         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

src/dawn_native/vulkan/DeviceVk.h

@@ -79,9 +79,6 @@ namespace dawn_native { namespace vulkan {
                                            BufferBase* destination,
                                            uint32_t destinationOffset,
                                            uint32_t size) override;
-
-        ResultOrError<DynamicUploader*> GetDynamicUploader() const;
-
       private:
         ResultOrError<BindGroupBase*> CreateBindGroupImpl(
             const BindGroupDescriptor* descriptor) override;
@@ -117,7 +114,6 @@ namespace dawn_native { namespace vulkan {
         uint32_t mQueueFamily = 0;
         VkQueue mQueue = VK_NULL_HANDLE;
 
-        std::unique_ptr<DynamicUploader> mDynamicUploader;
         std::unique_ptr<FencedDeleter> mDeleter;
         std::unique_ptr<MapRequestTracker> mMapRequestTracker;
         std::unique_ptr<MemoryAllocator> mMemoryAllocator;
@@ -148,9 +144,6 @@ namespace dawn_native { namespace vulkan {
         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

src/tests/end2end/BufferTests.cpp

@@ -180,10 +180,6 @@ TEST_P(BufferSetSubDataTests, SmallDataAtOffset) {
 
 // Stress test for many calls to SetSubData
 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());
-
     // 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
     // executing these commands in-flight. If this takes longer than ~2s, a device reset occurs and

src/tests/end2end/CopyTests.cpp

@@ -396,7 +396,7 @@ TEST_P(CopyTests_T2B, Texture2DArrayRegion) {
 
 // Test that copying texture 2D array mips with 256-byte aligned sizes works
 TEST_P(CopyTests_T2B, Texture2DArrayMip) {
-    // TODO(b-brber): Figure out why this test fails on Intel Linux.
+    // TODO(bryan.bernhart@intel.com): Figure out why this test fails on Intel Linux.
     // See https://bugs.chromium.org/p/dawn/issues/detail?id=101
     DAWN_SKIP_TEST_IF(IsLinux() && IsVulkan() && IsIntel());
     constexpr uint32_t kWidth = 256;

src/tests/unittests/RingBufferTests.cpp

@@ -33,7 +33,7 @@ namespace {
 class RingBufferTests : public testing::Test {
   protected:
     void SetUp() override {
-        // TODO(b-brber): Create this device through the adapter.
+        // TODO(bryan.bernhart@intel.com): Create this device through the adapter.
         mDevice = std::make_unique<null::Device>(/*adapter*/ nullptr);
     }
 
@@ -165,7 +165,7 @@ TEST_F(RingBufferTests, RingBufferSubAlloc) {
 
     // In this example, Tick(8) could not reclaim the wasted bytes. The wasted bytes
     // were add to F9's sub-allocation.
-    // TODO(b-brber): Decide if Tick(8) should free these wasted bytes.
+    // TODO(bryan.bernhart@intel.com): Decide if Tick(8) should free these wasted bytes.
 
     ASSERT_EQ(offset, 0u);
     ASSERT_EQ(buffer->GetUsedSize(), frameSizeInBytes * maxNumOfFrames);