Plumbs alignment offset through to RingBufferAllocator.
- By plumbing the offset through, the allocator can be smarter and only
allocate extra space for padding when necessary.
Bug: dawn:1985
Change-Id: I25f6762683cdd787e9a9d5ef3519231955a24914
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/151280
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Commit-Queue: Loko Kung <lokokung@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
diff --git a/src/dawn/native/DynamicUploader.cpp b/src/dawn/native/DynamicUploader.cpp
index f1de07b..980c441 100644
--- a/src/dawn/native/DynamicUploader.cpp
+++ b/src/dawn/native/DynamicUploader.cpp
@@ -32,7 +32,8 @@
}
ResultOrError<UploadHandle> DynamicUploader::AllocateInternal(uint64_t allocationSize,
- ExecutionSerial serial) {
+ ExecutionSerial serial,
+ uint64_t offsetAlignment) {
// Disable further sub-allocation should the request be too large.
if (allocationSize > kRingBufferSize) {
BufferDescriptor bufferDesc = {};
@@ -54,25 +55,20 @@
}
// Note: Validation ensures size is already aligned.
- // First-fit: find next smallest buffer large enough to satisfy the allocation request.
+ // First-fit: find next buffer large enough to satisfy the allocation request.
+ uint64_t startOffset = RingBufferAllocator::kInvalidOffset;
RingBuffer* targetRingBuffer = mRingBuffers.back().get();
for (auto& ringBuffer : mRingBuffers) {
- const RingBufferAllocator& ringBufferAllocator = ringBuffer->mAllocator;
+ RingBufferAllocator& ringBufferAllocator = ringBuffer->mAllocator;
// Prevent overflow.
DAWN_ASSERT(ringBufferAllocator.GetSize() >= ringBufferAllocator.GetUsedSize());
- const uint64_t remainingSize =
- ringBufferAllocator.GetSize() - ringBufferAllocator.GetUsedSize();
- if (allocationSize <= remainingSize) {
+ startOffset = ringBufferAllocator.Allocate(allocationSize, serial, offsetAlignment);
+ if (startOffset != RingBufferAllocator::kInvalidOffset) {
targetRingBuffer = ringBuffer.get();
break;
}
}
- uint64_t startOffset = RingBufferAllocator::kInvalidOffset;
- if (targetRingBuffer != nullptr) {
- startOffset = targetRingBuffer->mAllocator.Allocate(allocationSize, serial);
- }
-
// Upon failure, append a newly created ring buffer to fulfill the
// request.
if (startOffset == RingBufferAllocator::kInvalidOffset) {
@@ -126,19 +122,11 @@
mReleasedStagingBuffers.ClearUpTo(lastCompletedSerial);
}
-// TODO(dawn:512): Optimize this function so that it doesn't allocate additional memory
-// when it's not necessary.
ResultOrError<UploadHandle> DynamicUploader::Allocate(uint64_t allocationSize,
ExecutionSerial serial,
uint64_t offsetAlignment) {
DAWN_ASSERT(offsetAlignment > 0);
- UploadHandle uploadHandle;
- DAWN_TRY_ASSIGN(uploadHandle, AllocateInternal(allocationSize + offsetAlignment - 1, serial));
- uint64_t additionalOffset =
- Align(uploadHandle.startOffset, offsetAlignment) - uploadHandle.startOffset;
- uploadHandle.mappedBuffer = static_cast<uint8_t*>(uploadHandle.mappedBuffer) + additionalOffset;
- uploadHandle.startOffset += additionalOffset;
- return uploadHandle;
+ return AllocateInternal(allocationSize, serial, offsetAlignment);
}
bool DynamicUploader::ShouldFlush() {
diff --git a/src/dawn/native/DynamicUploader.h b/src/dawn/native/DynamicUploader.h
index e49ae0c..80b8f11 100644
--- a/src/dawn/native/DynamicUploader.h
+++ b/src/dawn/native/DynamicUploader.h
@@ -63,7 +63,9 @@
RingBufferAllocator mAllocator;
};
- ResultOrError<UploadHandle> AllocateInternal(uint64_t allocationSize, ExecutionSerial serial);
+ ResultOrError<UploadHandle> AllocateInternal(uint64_t allocationSize,
+ ExecutionSerial serial,
+ uint64_t offsetAlignment);
std::vector<std::unique_ptr<RingBuffer>> mRingBuffers;
SerialQueue<ExecutionSerial, Ref<BufferBase>> mReleasedStagingBuffers;
diff --git a/src/dawn/native/RingBufferAllocator.cpp b/src/dawn/native/RingBufferAllocator.cpp
index 3d02699..828d04dd 100644
--- a/src/dawn/native/RingBufferAllocator.cpp
+++ b/src/dawn/native/RingBufferAllocator.cpp
@@ -16,6 +16,8 @@
#include <utility>
+#include "dawn/common/Math.h"
+
// Note: Current RingBufferAllocator implementation uses two indices (start and end) to implement a
// circular queue. However, this approach defines a full queue when one element is still unused.
//
@@ -70,7 +72,9 @@
// queue, which identifies an existing (or new) frames-worth of resources. Internally, the
// ring-buffer maintains offsets of 3 "memory" states: Free, Reclaimed, and Used. This is done
// in FIFO order as older frames would free resources before newer ones.
-uint64_t RingBufferAllocator::Allocate(uint64_t allocationSize, ExecutionSerial serial) {
+uint64_t RingBufferAllocator::Allocate(uint64_t allocationSize,
+ ExecutionSerial serial,
+ uint64_t offsetAlignment) {
// Check if the buffer is full by comparing the used size.
// If the buffer is not split where waste occurs (e.g. cannot fit new sub-alloc in front), a
// subsequent sub-alloc could fail where the used size was previously adjusted to include
@@ -86,43 +90,45 @@
}
uint64_t startOffset = kInvalidOffset;
+ uint64_t currentRequestSize = 0u;
+
+ // Compute an alignment offset for the buffer if allocating at the end.
+ const uint64_t alignmentOffset = Align(mUsedEndOffset, offsetAlignment) - mUsedEndOffset;
+ const uint64_t alignedUsedEndOffset = mUsedEndOffset + alignmentOffset;
// Check if the buffer is NOT split (i.e sub-alloc on ends)
if (mUsedStartOffset <= mUsedEndOffset) {
// Order is important (try to sub-alloc at end first).
// This is due to FIFO order where sub-allocs are inserted from left-to-right (when not
// wrapped).
- if (mUsedEndOffset + allocationSize <= mMaxBlockSize) {
- startOffset = mUsedEndOffset;
- mUsedEndOffset += allocationSize;
- mUsedSize += allocationSize;
- mCurrentRequestSize += allocationSize;
+ if (alignedUsedEndOffset + allocationSize <= mMaxBlockSize) {
+ startOffset = alignedUsedEndOffset;
+ mUsedSize += allocationSize + alignmentOffset;
+ currentRequestSize = allocationSize + alignmentOffset;
} else if (allocationSize <= mUsedStartOffset) { // Try to sub-alloc at front.
// Count the space at the end so that a subsequent
// sub-alloc cannot fail when the buffer is full.
const uint64_t requestSize = (mMaxBlockSize - mUsedEndOffset) + allocationSize;
startOffset = 0;
- mUsedEndOffset = allocationSize;
mUsedSize += requestSize;
- mCurrentRequestSize += requestSize;
+ currentRequestSize = requestSize;
}
- } else if (mUsedEndOffset + allocationSize <=
- mUsedStartOffset) { // Otherwise, buffer is split where sub-alloc must be
- // in-between.
- startOffset = mUsedEndOffset;
- mUsedEndOffset += allocationSize;
- mUsedSize += allocationSize;
- mCurrentRequestSize += allocationSize;
+ } else if (alignedUsedEndOffset + allocationSize <= mUsedStartOffset) {
+ // Otherwise, buffer is split where sub-alloc must be in-between.
+ startOffset = alignedUsedEndOffset;
+ mUsedSize += allocationSize + alignmentOffset;
+ currentRequestSize = allocationSize + alignmentOffset;
}
if (startOffset != kInvalidOffset) {
+ mUsedEndOffset = startOffset + allocationSize;
+
Request request;
request.endOffset = mUsedEndOffset;
- request.size = mCurrentRequestSize;
+ request.size = currentRequestSize;
mInflightRequests.Enqueue(std::move(request), serial);
- mCurrentRequestSize = 0; // reset
}
return startOffset;
diff --git a/src/dawn/native/RingBufferAllocator.h b/src/dawn/native/RingBufferAllocator.h
index 6aeb142..e3b6840 100644
--- a/src/dawn/native/RingBufferAllocator.h
+++ b/src/dawn/native/RingBufferAllocator.h
@@ -33,7 +33,9 @@
RingBufferAllocator& operator=(const RingBufferAllocator&);
- uint64_t Allocate(uint64_t allocationSize, ExecutionSerial serial);
+ uint64_t Allocate(uint64_t allocationSize,
+ ExecutionSerial serial,
+ uint64_t offsetAlignment = 1);
void Deallocate(ExecutionSerial lastCompletedSerial);
uint64_t GetSize() const;
@@ -55,8 +57,6 @@
uint64_t mUsedStartOffset = 0; // Head of used sub-alloc requests (in bytes).
uint64_t mMaxBlockSize = 0; // Max size of the ring buffer (in bytes).
uint64_t mUsedSize = 0; // Size of the sub-alloc requests (in bytes) of the ring buffer.
- uint64_t mCurrentRequestSize =
- 0; // Size of the sub-alloc requests (in bytes) of the current serial.
};
} // namespace dawn::native
diff --git a/src/dawn/tests/end2end/QueueTests.cpp b/src/dawn/tests/end2end/QueueTests.cpp
index 0cbd7c7..f38fdb4 100644
--- a/src/dawn/tests/end2end/QueueTests.cpp
+++ b/src/dawn/tests/end2end/QueueTests.cpp
@@ -174,6 +174,19 @@
EXPECT_BUFFER_U32_RANGE_EQ(expectedData.data(), buffer, 0, kElements);
}
+// Test using the max buffer size. Regression test for dawn:1985. We don't bother validating the
+// results for this case since that would take a lot longer, just that there are no errors.
+TEST_P(QueueWriteBufferTests, MaxBufferSizeWriteBuffer) {
+ uint32_t maxBufferSize = GetSupportedLimits().limits.maxBufferSize;
+ wgpu::BufferDescriptor descriptor;
+ descriptor.size = maxBufferSize;
+ descriptor.usage = wgpu::BufferUsage::CopySrc | wgpu::BufferUsage::CopyDst;
+ wgpu::Buffer buffer = device.CreateBuffer(&descriptor);
+
+ std::vector<uint8_t> data(maxBufferSize);
+ queue.WriteBuffer(buffer, 0, data.data(), maxBufferSize);
+}
+
// Test a special code path: writing when dynamic uploader already contatins some unaligned
// data, it might be necessary to use a ring buffer with properly aligned offset.
TEST_P(QueueWriteBufferTests, UnalignedDynamicUploader) {
