Skip to content

Commit

Permalink
gpu: Add option for transfer buffer to gracefully fail on shmem OOM
Browse files Browse the repository at this point in the history
The WebGPU API provides facilities to create mapped GPUBuffers
backed by shared memory. This makes it very easy for JavaScript to
make large transfer buffer allocations which fail allocation and
instantly lose the context.
This CL adds an option to CommandBuffer::CreateTransferBuffer and
MappedMemoryManager::Alloc to not lose the context on OOM but
instead fail gracefully.

Bug: dawn:450
Change-Id: I7311dd2ab685a150574056fbe9df5fa1724e4b7c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2310831
Reviewed-by: Bill Budge <bbudge@chromium.org>
Reviewed-by: Kenneth Russell <kbr@chromium.org>
Reviewed-by: Zhenyao Mo <zmo@chromium.org>
Commit-Queue: Austin Eng <enga@chromium.org>
Cr-Commit-Position: refs/heads/master@{#791071}
  • Loading branch information
austinEng authored and Commit Bot committed Jul 22, 2020
1 parent 5e97862 commit a207388
Show file tree
Hide file tree
Showing 21 changed files with 193 additions and 67 deletions.
7 changes: 5 additions & 2 deletions gpu/command_buffer/client/buffer_tracker_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,11 @@ class MockClientCommandBufferImpl : public MockClientCommandBuffer {
context_lost_(false) {}
~MockClientCommandBufferImpl() override = default;

scoped_refptr<gpu::Buffer> CreateTransferBuffer(uint32_t size,
int32_t* id) override {
scoped_refptr<gpu::Buffer> CreateTransferBuffer(
uint32_t size,
int32_t* id,
TransferBufferAllocationOption option =
TransferBufferAllocationOption::kLoseContextOnOOM) override {
if (context_lost_) {
*id = -1;
return nullptr;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,11 @@ class FakeCommandBuffer : public CommandBuffer {
return State();
}
void SetGetBuffer(int32_t transfer_buffer_id) override { NOTREACHED(); }
scoped_refptr<gpu::Buffer> CreateTransferBuffer(uint32_t size,
int32_t* id) override {
scoped_refptr<gpu::Buffer> CreateTransferBuffer(
uint32_t size,
int32_t* id,
TransferBufferAllocationOption option =
TransferBufferAllocationOption::kLoseContextOnOOM) override {
*id = next_id_++;
active_ids_.insert(*id);
return MakeMemoryBuffer(size);
Expand Down
3 changes: 2 additions & 1 deletion gpu/command_buffer/client/client_test_helper.cc
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,8 @@ void MockClientCommandBuffer::SetGetBuffer(int transfer_buffer_id) {

scoped_refptr<gpu::Buffer> MockClientCommandBuffer::CreateTransferBuffer(
uint32_t size,
int32_t* id) {
int32_t* id,
TransferBufferAllocationOption option) {
return CreateTransferBufferHelper(size, id);
}

Expand Down
7 changes: 5 additions & 2 deletions gpu/command_buffer/client/client_test_helper.h
Original file line number Diff line number Diff line change
Expand Up @@ -67,8 +67,11 @@ class MockClientCommandBuffer : public CommandBuffer,
int32_t start,
int32_t end) override;
void SetGetBuffer(int transfer_buffer_id) override;
scoped_refptr<gpu::Buffer> CreateTransferBuffer(uint32_t size,
int32_t* id) override;
scoped_refptr<gpu::Buffer> CreateTransferBuffer(
uint32_t size,
int32_t* id,
TransferBufferAllocationOption option =
TransferBufferAllocationOption::kLoseContextOnOOM) override;

// This is so we can use all the gmock functions when Flush is called.
MOCK_METHOD0(OnFlush, void());
Expand Down
5 changes: 3 additions & 2 deletions gpu/command_buffer/client/command_buffer_direct_locked.cc
Original file line number Diff line number Diff line change
Expand Up @@ -43,12 +43,13 @@ CommandBuffer::State CommandBufferDirectLocked::WaitForGetOffsetInRange(

scoped_refptr<Buffer> CommandBufferDirectLocked::CreateTransferBuffer(
uint32_t size,
int32_t* id) {
int32_t* id,
TransferBufferAllocationOption option) {
if (fail_create_transfer_buffer_) {
*id = -1;
return nullptr;
} else {
return CommandBufferDirect::CreateTransferBuffer(size, id);
return CommandBufferDirect::CreateTransferBuffer(size, id, option);
}
}

Expand Down
7 changes: 5 additions & 2 deletions gpu/command_buffer/client/command_buffer_direct_locked.h
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,11 @@ class CommandBufferDirectLocked : public CommandBufferDirect {
CommandBuffer::State WaitForGetOffsetInRange(uint32_t set_get_buffer_count,
int32_t start,
int32_t end) override;
scoped_refptr<Buffer> CreateTransferBuffer(uint32_t size,
int32_t* id) override;
scoped_refptr<Buffer> CreateTransferBuffer(
uint32_t size,
int32_t* id,
TransferBufferAllocationOption option =
TransferBufferAllocationOption::kLoseContextOnOOM) override;

void LockFlush() { flush_locked_ = true; }

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -146,8 +146,9 @@ void* DawnClientMemoryTransferService::AllocateHandle(
handle->size = static_cast<uint32_t>(size);

DCHECK(mapped_memory_);
return mapped_memory_->Alloc(handle->size, &handle->shm_id,
&handle->shm_offset);
return mapped_memory_->Alloc(
handle->size, &handle->shm_id, &handle->shm_offset,
TransferBufferAllocationOption::kReturnNullOnOOM);
}

void DawnClientMemoryTransferService::MarkHandleFree(void* ptr) {
Expand Down
5 changes: 3 additions & 2 deletions gpu/command_buffer/client/mapped_memory.cc
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,8 @@ MappedMemoryManager::~MappedMemoryManager() {

void* MappedMemoryManager::Alloc(unsigned int size,
int32_t* shm_id,
unsigned int* shm_offset) {
unsigned int* shm_offset,
TransferBufferAllocationOption option) {
DCHECK(shm_id);
DCHECK(shm_offset);
if (size <= allocated_memory_) {
Expand Down Expand Up @@ -113,7 +114,7 @@ void* MappedMemoryManager::Alloc(unsigned int size,

int32_t id = -1;
scoped_refptr<gpu::Buffer> shm =
cmd_buf->CreateTransferBuffer(safe_chunk_size, &id);
cmd_buf->CreateTransferBuffer(safe_chunk_size, &id, option);
if (id < 0)
return nullptr;
DCHECK(shm.get());
Expand Down
11 changes: 10 additions & 1 deletion gpu/command_buffer/client/mapped_memory.h
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
#include "base/trace_event/memory_dump_provider.h"
#include "gpu/command_buffer/client/fenced_allocator.h"
#include "gpu/command_buffer/common/buffer.h"
#include "gpu/command_buffer/common/constants.h"
#include "gpu/gpu_export.h"

namespace gpu {
Expand Down Expand Up @@ -150,9 +151,17 @@ class GPU_EXPORT MappedMemoryManager {
// size: size of memory to allocate.
// shm_id: pointer to variable to receive the shared memory id.
// shm_offset: pointer to variable to receive the shared memory offset.
// option: defaults to kLoseContextOnOOM, but may be kReturnNullOnOOM.
// Passing kReturnNullOnOOM will gracefully fail and return nullptr
// on OOM instead of losing the context. Callers should be careful
// to check error conditions.
// Returns:
// pointer to allocated block of memory. nullptr if failure.
void* Alloc(uint32_t size, int32_t* shm_id, uint32_t* shm_offset);
void* Alloc(uint32_t size,
int32_t* shm_id,
uint32_t* shm_offset,
TransferBufferAllocationOption option =
TransferBufferAllocationOption::kLoseContextOnOOM);

// Frees a block of memory.
//
Expand Down
75 changes: 40 additions & 35 deletions gpu/command_buffer/client/transfer_buffer_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -232,12 +232,17 @@ class MockClientCommandBufferCanFail : public MockClientCommandBufferMockFlush {
MockClientCommandBufferCanFail() = default;
~MockClientCommandBufferCanFail() override = default;

MOCK_METHOD2(CreateTransferBuffer,
scoped_refptr<Buffer>(uint32_t size, int32_t* id));

scoped_refptr<gpu::Buffer> RealCreateTransferBuffer(uint32_t size,
int32_t* id) {
return MockClientCommandBufferMockFlush::CreateTransferBuffer(size, id);
MOCK_METHOD3(CreateTransferBuffer,
scoped_refptr<Buffer>(uint32_t size,
int32_t* id,
TransferBufferAllocationOption option));

scoped_refptr<gpu::Buffer> RealCreateTransferBuffer(
uint32_t size,
int32_t* id,
TransferBufferAllocationOption option) {
return MockClientCommandBufferMockFlush::CreateTransferBuffer(size, id,
option);
}
};

Expand Down Expand Up @@ -274,10 +279,10 @@ void TransferBufferExpandContractTest::SetUp() {
command_buffer_->SetTokenForSetGetBuffer(0);

EXPECT_CALL(*command_buffer(),
CreateTransferBuffer(kCommandBufferSizeBytes, _))
.WillOnce(Invoke(
command_buffer(),
&MockClientCommandBufferCanFail::RealCreateTransferBuffer))
CreateTransferBuffer(kCommandBufferSizeBytes, _, _))
.WillOnce(
Invoke(command_buffer(),
&MockClientCommandBufferCanFail::RealCreateTransferBuffer))
.RetiresOnSaturation();

helper_.reset(new CommandBufferHelper(command_buffer()));
Expand All @@ -287,10 +292,10 @@ void TransferBufferExpandContractTest::SetUp() {
transfer_buffer_id_ = command_buffer()->GetNextFreeTransferBufferId();

EXPECT_CALL(*command_buffer(),
CreateTransferBuffer(kStartTransferBufferSize, _))
.WillOnce(Invoke(
command_buffer(),
&MockClientCommandBufferCanFail::RealCreateTransferBuffer))
CreateTransferBuffer(kStartTransferBufferSize, _, _))
.WillOnce(
Invoke(command_buffer(),
&MockClientCommandBufferCanFail::RealCreateTransferBuffer))
.RetiresOnSaturation();

transfer_buffer_.reset(new TransferBuffer(helper_.get()));
Expand Down Expand Up @@ -341,7 +346,7 @@ TEST_F(TransferBufferExpandContractTest, ExpandWithSmallAllocations) {
EXPECT_CALL(*command_buffer(), OrderingBarrier(_))
.Times(1)
.RetiresOnSaturation();
EXPECT_CALL(*command_buffer(), CreateTransferBuffer(size, _))
EXPECT_CALL(*command_buffer(), CreateTransferBuffer(size, _, _))
.WillOnce(
Invoke(command_buffer(),
&MockClientCommandBufferCanFail::RealCreateTransferBuffer))
Expand Down Expand Up @@ -438,7 +443,7 @@ TEST_F(TransferBufferExpandContractTest, ExpandWithLargeAllocations) {
EXPECT_CALL(*command_buffer(), OrderingBarrier(_))
.Times(1)
.RetiresOnSaturation();
EXPECT_CALL(*command_buffer(), CreateTransferBuffer(size, _))
EXPECT_CALL(*command_buffer(), CreateTransferBuffer(size, _, _))
.WillOnce(
Invoke(command_buffer(),
&MockClientCommandBufferCanFail::RealCreateTransferBuffer))
Expand Down Expand Up @@ -488,7 +493,7 @@ TEST_F(TransferBufferExpandContractTest, ShrinkRingBuffer) {
EXPECT_CALL(*command_buffer(), OrderingBarrier(_))
.Times(1)
.RetiresOnSaturation();
EXPECT_CALL(*command_buffer(), CreateTransferBuffer(size, _))
EXPECT_CALL(*command_buffer(), CreateTransferBuffer(size, _, _))
.WillOnce(
Invoke(command_buffer(),
&MockClientCommandBufferCanFail::RealCreateTransferBuffer))
Expand Down Expand Up @@ -536,15 +541,15 @@ TEST_F(TransferBufferExpandContractTest, Contract) {

// Try to allocate again, fail first request
EXPECT_CALL(*command_buffer(),
CreateTransferBuffer(kStartTransferBufferSize, _))
CreateTransferBuffer(kStartTransferBufferSize, _, _))
.WillOnce(
DoAll(SetArgPointee<1>(-1), Return(scoped_refptr<gpu::Buffer>())))
DoAll(SetArgPointee<1>(-1), Return(scoped_refptr<gpu::Buffer>())))
.RetiresOnSaturation();
EXPECT_CALL(*command_buffer(),
CreateTransferBuffer(kMinTransferBufferSize, _))
.WillOnce(Invoke(
command_buffer(),
&MockClientCommandBufferCanFail::RealCreateTransferBuffer))
CreateTransferBuffer(kMinTransferBufferSize, _, _))
.WillOnce(
Invoke(command_buffer(),
&MockClientCommandBufferCanFail::RealCreateTransferBuffer))
.RetiresOnSaturation();

const uint32_t kSize1 = 256 - kStartingOffset;
Expand All @@ -569,10 +574,10 @@ TEST_F(TransferBufferExpandContractTest, Contract) {

// Try to allocate again,
EXPECT_CALL(*command_buffer(),
CreateTransferBuffer(kMinTransferBufferSize, _))
.WillOnce(Invoke(
command_buffer(),
&MockClientCommandBufferCanFail::RealCreateTransferBuffer))
CreateTransferBuffer(kMinTransferBufferSize, _, _))
.WillOnce(
Invoke(command_buffer(),
&MockClientCommandBufferCanFail::RealCreateTransferBuffer))
.RetiresOnSaturation();

ptr = transfer_buffer_->AllocUpTo(kSize1, &size_allocated);
Expand All @@ -595,13 +600,13 @@ TEST_F(TransferBufferExpandContractTest, OutOfMemory) {
EXPECT_FALSE(transfer_buffer_->HaveBuffer());

// Try to allocate again, fail both requests.
EXPECT_CALL(*command_buffer(), CreateTransferBuffer(_, _))
EXPECT_CALL(*command_buffer(), CreateTransferBuffer(_, _, _))
.WillOnce(
DoAll(SetArgPointee<1>(-1), Return(scoped_refptr<gpu::Buffer>())))
DoAll(SetArgPointee<1>(-1), Return(scoped_refptr<gpu::Buffer>())))
.WillOnce(
DoAll(SetArgPointee<1>(-1), Return(scoped_refptr<gpu::Buffer>())))
DoAll(SetArgPointee<1>(-1), Return(scoped_refptr<gpu::Buffer>())))
.WillOnce(
DoAll(SetArgPointee<1>(-1), Return(scoped_refptr<gpu::Buffer>())))
DoAll(SetArgPointee<1>(-1), Return(scoped_refptr<gpu::Buffer>())))
.RetiresOnSaturation();

const uint32_t kSize1 = 512 - kStartingOffset;
Expand All @@ -625,10 +630,10 @@ TEST_F(TransferBufferExpandContractTest, ReallocsToDefault) {

// See that it gets reallocated.
EXPECT_CALL(*command_buffer(),
CreateTransferBuffer(kStartTransferBufferSize, _))
.WillOnce(Invoke(
command_buffer(),
&MockClientCommandBufferCanFail::RealCreateTransferBuffer))
CreateTransferBuffer(kStartTransferBufferSize, _, _))
.WillOnce(
Invoke(command_buffer(),
&MockClientCommandBufferCanFail::RealCreateTransferBuffer))
.RetiresOnSaturation();
EXPECT_EQ(transfer_buffer_id_, transfer_buffer_->GetShmId());
EXPECT_TRUE(transfer_buffer_->HaveBuffer());
Expand Down
11 changes: 9 additions & 2 deletions gpu/command_buffer/common/command_buffer.h
Original file line number Diff line number Diff line change
Expand Up @@ -110,8 +110,15 @@ class GPU_EXPORT CommandBuffer {

// Create a transfer buffer of the given size. Returns its ID or -1 on
// error.
virtual scoped_refptr<gpu::Buffer> CreateTransferBuffer(uint32_t size,
int32_t* id) = 0;
// The |option| argument defaults to kLoseContextOnOOM, but may be
// kReturnNullOnOOM. Passing kReturnNullOnOOM will gracefully fail and return
// nullptr on OOM instead of losing the context. Callers should be careful
// to check error conditions.
virtual scoped_refptr<gpu::Buffer> CreateTransferBuffer(
uint32_t size,
int32_t* id,
TransferBufferAllocationOption option =
TransferBufferAllocationOption::kLoseContextOnOOM) = 0;

// Destroy a transfer buffer. The ID must be positive.
// An ordering barrier must be placed after any commands that use the buffer
Expand Down
5 changes: 5 additions & 0 deletions gpu/command_buffer/common/constants.h
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,11 @@ enum CommandBufferNamespace : int8_t {
NUM_COMMAND_BUFFER_NAMESPACES
};

enum class TransferBufferAllocationOption : int8_t {
kLoseContextOnOOM,
kReturnNullOnOOM,
};

} // namespace gpu

#endif // GPU_COMMAND_BUFFER_COMMON_CONSTANTS_H_
6 changes: 4 additions & 2 deletions gpu/command_buffer/service/command_buffer_direct.cc
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,10 @@ void CommandBufferDirect::SetGetBuffer(int32_t transfer_buffer_id) {
service_.SetGetBuffer(transfer_buffer_id);
}

scoped_refptr<Buffer> CommandBufferDirect::CreateTransferBuffer(uint32_t size,
int32_t* id) {
scoped_refptr<Buffer> CommandBufferDirect::CreateTransferBuffer(
uint32_t size,
int32_t* id,
TransferBufferAllocationOption option) {
return service_.CreateTransferBuffer(size, id);
}

Expand Down
7 changes: 5 additions & 2 deletions gpu/command_buffer/service/command_buffer_direct.h
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,11 @@ class GPU_EXPORT CommandBufferDirect : public CommandBuffer,
int32_t start,
int32_t end) override;
void SetGetBuffer(int32_t transfer_buffer_id) override;
scoped_refptr<Buffer> CreateTransferBuffer(uint32_t size,
int32_t* id) override;
scoped_refptr<Buffer> CreateTransferBuffer(
uint32_t size,
int32_t* id,
TransferBufferAllocationOption option =
TransferBufferAllocationOption::kLoseContextOnOOM) override;
void DestroyTransferBuffer(int32_t id) override;

// CommandBufferServiceClient implementation:
Expand Down
6 changes: 4 additions & 2 deletions gpu/ipc/client/command_buffer_proxy_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -359,7 +359,8 @@ void CommandBufferProxyImpl::SetGetBuffer(int32_t shm_id) {

scoped_refptr<gpu::Buffer> CommandBufferProxyImpl::CreateTransferBuffer(
uint32_t size,
int32_t* id) {
int32_t* id,
TransferBufferAllocationOption option) {
CheckLock();
base::AutoLock lock(last_state_lock_);
*id = -1;
Expand All @@ -371,7 +372,8 @@ scoped_refptr<gpu::Buffer> CommandBufferProxyImpl::CreateTransferBuffer(
std::tie(shared_memory_region, shared_memory_mapping) =
AllocateAndMapSharedMemory(size);
if (!shared_memory_mapping.IsValid()) {
if (last_state_.error == gpu::error::kNoError)
if (last_state_.error == gpu::error::kNoError &&
option != TransferBufferAllocationOption::kReturnNullOnOOM)
OnClientError(gpu::error::kOutOfBounds);
return nullptr;
}
Expand Down
7 changes: 5 additions & 2 deletions gpu/ipc/client/command_buffer_proxy_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -101,8 +101,11 @@ class GPU_EXPORT CommandBufferProxyImpl : public gpu::CommandBuffer,
int32_t start,
int32_t end) override;
void SetGetBuffer(int32_t shm_id) override;
scoped_refptr<gpu::Buffer> CreateTransferBuffer(uint32_t size,
int32_t* id) override;
scoped_refptr<gpu::Buffer> CreateTransferBuffer(
uint32_t size,
int32_t* id,
TransferBufferAllocationOption option =
TransferBufferAllocationOption::kLoseContextOnOOM) override;
void DestroyTransferBuffer(int32_t id) override;

// gpu::GpuControl implementation:
Expand Down
Loading

0 comments on commit a207388

Please sign in to comment.