Commit b63225bb authored by James Darpinian's avatar James Darpinian Committed by Commit Bot

Revert "GPU: Remove transfer buffer autoflushing."

This reverts commit dcfaac01.

Reason for revert: Regressed canvas 2d performance in some MotionMark
tests. Will redo as a WebGL-specific change.

Original change's description:
> GPU: Remove transfer buffer autoflushing.
> 
> This is change #2 of 3 reducing context switching during WebGL
> rendering. On tiled rendering GPUs context switches are slow and can
> even cause multisampling artifacts. Every time the command buffer is
> flushed, a context switch is possible.
> 
> Before, the transfer buffer would flush the command buffer after an
> arbitrary number of bytes were sent. This change removes that threshold.
> Any performance benefit of early flushing is killed by the additional
> context switching.
> 
> Bug: 828363,835353
> Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel
> Change-Id: I8f71668d4b976332820d290868282d41286ab4fe
> Reviewed-on: https://chromium-review.googlesource.com/1077568
> Commit-Queue: James Darpinian <jdarpinian@chromium.org>
> Reviewed-by: Antoine Labour <piman@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#563404}

TBR=piman@chromium.org,jdarpinian@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 828363, 835353, 849348
Change-Id: Iba27bae33f5395333f54db4bea4e79e41cca7e31
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel
Reviewed-on: https://chromium-review.googlesource.com/1087370
Commit-Queue: James Darpinian <jdarpinian@chromium.org>
Commit-Queue: Kenneth Russell <kbr@chromium.org>
Reviewed-by: default avatarKenneth Russell <kbr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#565006}
parent b34141f7
...@@ -156,7 +156,7 @@ gpu::ContextResult ImplementationBase::Initialize( ...@@ -156,7 +156,7 @@ gpu::ContextResult ImplementationBase::Initialize(
if (!transfer_buffer_->Initialize( if (!transfer_buffer_->Initialize(
limits.start_transfer_buffer_size, kStartingOffset, limits.start_transfer_buffer_size, kStartingOffset,
limits.min_transfer_buffer_size, limits.max_transfer_buffer_size, limits.min_transfer_buffer_size, limits.max_transfer_buffer_size,
kAlignment)) { kAlignment, kSizeToFlush)) {
// TransferBuffer::Initialize doesn't fail for transient reasons such as if // TransferBuffer::Initialize doesn't fail for transient reasons such as if
// the context was lost. See http://crrev.com/c/720269 // the context was lost. See http://crrev.com/c/720269
LOG(ERROR) << "ContextResult::kFatalFailure: " LOG(ERROR) << "ContextResult::kFatalFailure: "
......
...@@ -51,6 +51,9 @@ class GLES2_IMPL_EXPORT ImplementationBase ...@@ -51,6 +51,9 @@ class GLES2_IMPL_EXPORT ImplementationBase
// used for testing only. If more things are reseved add them here. // used for testing only. If more things are reseved add them here.
static const unsigned int kStartingOffset = kMaxSizeOfSimpleResult; static const unsigned int kStartingOffset = kMaxSizeOfSimpleResult;
// Size in bytes to issue async flush for transfer buffer.
static const unsigned int kSizeToFlush = 256 * 1024;
// Alignment of allocations. // Alignment of allocations.
static const unsigned int kAlignment = 16; static const unsigned int kAlignment = 16;
......
...@@ -44,7 +44,8 @@ bool MockTransferBuffer::Initialize(unsigned int starting_buffer_size, ...@@ -44,7 +44,8 @@ bool MockTransferBuffer::Initialize(unsigned int starting_buffer_size,
unsigned int result_size, unsigned int result_size,
unsigned int /* min_buffer_size */, unsigned int /* min_buffer_size */,
unsigned int /* max_buffer_size */, unsigned int /* max_buffer_size */,
unsigned int alignment) { unsigned int alignment,
unsigned int /* size_to_flush */) {
// Just check they match. // Just check they match.
return size_ == starting_buffer_size && result_size_ == result_size && return size_ == starting_buffer_size && result_size_ == result_size &&
alignment_ == alignment && !initialize_fail_; alignment_ == alignment && !initialize_fail_;
......
...@@ -35,7 +35,8 @@ class MockTransferBuffer : public TransferBufferInterface { ...@@ -35,7 +35,8 @@ class MockTransferBuffer : public TransferBufferInterface {
unsigned int result_size, unsigned int result_size,
unsigned int /* min_buffer_size */, unsigned int /* min_buffer_size */,
unsigned int /* max_buffer_size */, unsigned int /* max_buffer_size */,
unsigned int alignment) override; unsigned int alignment,
unsigned int size_to_flush) override;
int GetShmId() override; int GetShmId() override;
void* GetResultBuffer() override; void* GetResultBuffer() override;
int GetResultOffset() override; int GetResultOffset() override;
......
...@@ -25,6 +25,8 @@ TransferBuffer::TransferBuffer( ...@@ -25,6 +25,8 @@ TransferBuffer::TransferBuffer(
min_buffer_size_(0), min_buffer_size_(0),
max_buffer_size_(0), max_buffer_size_(0),
alignment_(0), alignment_(0),
size_to_flush_(0),
bytes_since_last_flush_(0),
buffer_id_(-1), buffer_id_(-1),
result_buffer_(NULL), result_buffer_(NULL),
result_shm_offset_(0), result_shm_offset_(0),
...@@ -43,16 +45,19 @@ base::SharedMemoryHandle TransferBuffer::shared_memory_handle() const { ...@@ -43,16 +45,19 @@ base::SharedMemoryHandle TransferBuffer::shared_memory_handle() const {
return buffer_->backing()->shared_memory_handle(); return buffer_->backing()->shared_memory_handle();
} }
bool TransferBuffer::Initialize(unsigned int default_buffer_size, bool TransferBuffer::Initialize(
unsigned int result_size, unsigned int default_buffer_size,
unsigned int min_buffer_size, unsigned int result_size,
unsigned int max_buffer_size, unsigned int min_buffer_size,
unsigned int alignment) { unsigned int max_buffer_size,
unsigned int alignment,
unsigned int size_to_flush) {
result_size_ = result_size; result_size_ = result_size;
default_buffer_size_ = default_buffer_size; default_buffer_size_ = default_buffer_size;
min_buffer_size_ = min_buffer_size; min_buffer_size_ = min_buffer_size;
max_buffer_size_ = max_buffer_size; max_buffer_size_ = max_buffer_size;
alignment_ = alignment; alignment_ = alignment;
size_to_flush_ = size_to_flush;
ReallocateRingBuffer(default_buffer_size_ - result_size); ReallocateRingBuffer(default_buffer_size_ - result_size);
return HaveBuffer(); return HaveBuffer();
} }
...@@ -67,6 +72,7 @@ void TransferBuffer::Free() { ...@@ -67,6 +72,7 @@ void TransferBuffer::Free() {
result_buffer_ = NULL; result_buffer_ = NULL;
result_shm_offset_ = 0; result_shm_offset_ = 0;
ring_buffer_.reset(); ring_buffer_.reset();
bytes_since_last_flush_ = 0;
} }
} }
...@@ -85,6 +91,10 @@ void TransferBuffer::DiscardBlock(void* p) { ...@@ -85,6 +91,10 @@ void TransferBuffer::DiscardBlock(void* p) {
void TransferBuffer::FreePendingToken(void* p, unsigned int token) { void TransferBuffer::FreePendingToken(void* p, unsigned int token) {
ring_buffer_->FreePendingToken(p, token); ring_buffer_->FreePendingToken(p, token);
if (bytes_since_last_flush_ >= size_to_flush_ && size_to_flush_ > 0) {
helper_->Flush();
bytes_since_last_flush_ = 0;
}
} }
unsigned int TransferBuffer::GetSize() const { unsigned int TransferBuffer::GetSize() const {
...@@ -158,6 +168,7 @@ void* TransferBuffer::AllocUpTo( ...@@ -158,6 +168,7 @@ void* TransferBuffer::AllocUpTo(
unsigned int max_size = ring_buffer_->GetLargestFreeOrPendingSize(); unsigned int max_size = ring_buffer_->GetLargestFreeOrPendingSize();
*size_allocated = std::min(max_size, size); *size_allocated = std::min(max_size, size);
bytes_since_last_flush_ += *size_allocated;
return ring_buffer_->Alloc(*size_allocated); return ring_buffer_->Alloc(*size_allocated);
} }
...@@ -173,6 +184,7 @@ void* TransferBuffer::Alloc(unsigned int size) { ...@@ -173,6 +184,7 @@ void* TransferBuffer::Alloc(unsigned int size) {
return NULL; return NULL;
} }
bytes_since_last_flush_ += size;
return ring_buffer_->Alloc(size); return ring_buffer_->Alloc(size);
} }
......
...@@ -36,11 +36,13 @@ class GPU_EXPORT TransferBufferInterface { ...@@ -36,11 +36,13 @@ class GPU_EXPORT TransferBufferInterface {
// Otherwise, this returns an invalid handle. // Otherwise, this returns an invalid handle.
virtual base::SharedMemoryHandle shared_memory_handle() const = 0; virtual base::SharedMemoryHandle shared_memory_handle() const = 0;
virtual bool Initialize(unsigned int buffer_size, virtual bool Initialize(
unsigned int result_size, unsigned int buffer_size,
unsigned int min_buffer_size, unsigned int result_size,
unsigned int max_buffer_size, unsigned int min_buffer_size,
unsigned int alignment) = 0; unsigned int max_buffer_size,
unsigned int alignment,
unsigned int size_to_flush) = 0;
virtual int GetShmId() = 0; virtual int GetShmId() = 0;
virtual void* GetResultBuffer() = 0; virtual void* GetResultBuffer() = 0;
...@@ -84,7 +86,8 @@ class GPU_EXPORT TransferBuffer : public TransferBufferInterface { ...@@ -84,7 +86,8 @@ class GPU_EXPORT TransferBuffer : public TransferBufferInterface {
unsigned int result_size, unsigned int result_size,
unsigned int min_buffer_size, unsigned int min_buffer_size,
unsigned int max_buffer_size, unsigned int max_buffer_size,
unsigned int alignment) override; unsigned int alignment,
unsigned int size_to_flush) override;
int GetShmId() override; int GetShmId() override;
void* GetResultBuffer() override; void* GetResultBuffer() override;
int GetResultOffset() override; int GetResultOffset() override;
...@@ -128,6 +131,12 @@ class GPU_EXPORT TransferBuffer : public TransferBufferInterface { ...@@ -128,6 +131,12 @@ class GPU_EXPORT TransferBuffer : public TransferBufferInterface {
// alignment for allocations // alignment for allocations
unsigned int alignment_; unsigned int alignment_;
// Size at which to do an async flush. 0 = never.
unsigned int size_to_flush_;
// Number of bytes since we last flushed.
unsigned int bytes_since_last_flush_;
// the current buffer. // the current buffer.
scoped_refptr<gpu::Buffer> buffer_; scoped_refptr<gpu::Buffer> buffer_;
......
...@@ -44,10 +44,14 @@ class TransferBufferTest : public testing::Test { ...@@ -44,10 +44,14 @@ class TransferBufferTest : public testing::Test {
void SetUp() override; void SetUp() override;
void TearDown() override; void TearDown() override;
virtual void Initialize() { virtual void Initialize(unsigned int size_to_flush) {
ASSERT_TRUE(transfer_buffer_->Initialize( ASSERT_TRUE(transfer_buffer_->Initialize(
kTransferBufferSize, kStartingOffset, kTransferBufferSize, kTransferBufferSize,
kTransferBufferSize, kAlignment)); kStartingOffset,
kTransferBufferSize,
kTransferBufferSize,
kAlignment,
size_to_flush));
} }
MockClientCommandBufferMockFlush* command_buffer() const { MockClientCommandBufferMockFlush* command_buffer() const {
...@@ -97,7 +101,7 @@ const size_t TransferBufferTest::kTransferBufferSize; ...@@ -97,7 +101,7 @@ const size_t TransferBufferTest::kTransferBufferSize;
#endif #endif
TEST_F(TransferBufferTest, Basic) { TEST_F(TransferBufferTest, Basic) {
Initialize(); Initialize(0);
EXPECT_TRUE(transfer_buffer_->HaveBuffer()); EXPECT_TRUE(transfer_buffer_->HaveBuffer());
EXPECT_EQ(transfer_buffer_id_, transfer_buffer_->GetShmId()); EXPECT_EQ(transfer_buffer_id_, transfer_buffer_->GetShmId());
EXPECT_EQ( EXPECT_EQ(
...@@ -108,7 +112,7 @@ TEST_F(TransferBufferTest, Basic) { ...@@ -108,7 +112,7 @@ TEST_F(TransferBufferTest, Basic) {
} }
TEST_F(TransferBufferTest, Free) { TEST_F(TransferBufferTest, Free) {
Initialize(); Initialize(0);
EXPECT_TRUE(transfer_buffer_->HaveBuffer()); EXPECT_TRUE(transfer_buffer_->HaveBuffer());
EXPECT_EQ(transfer_buffer_id_, transfer_buffer_->GetShmId()); EXPECT_EQ(transfer_buffer_id_, transfer_buffer_->GetShmId());
EXPECT_NE(base::UnguessableToken(), EXPECT_NE(base::UnguessableToken(),
...@@ -200,7 +204,7 @@ TEST_F(TransferBufferTest, Free) { ...@@ -200,7 +204,7 @@ TEST_F(TransferBufferTest, Free) {
} }
TEST_F(TransferBufferTest, TooLargeAllocation) { TEST_F(TransferBufferTest, TooLargeAllocation) {
Initialize(); Initialize(0);
// Check that we can't allocate large than max size. // Check that we can't allocate large than max size.
void* ptr = transfer_buffer_->Alloc(kTransferBufferSize + 1); void* ptr = transfer_buffer_->Alloc(kTransferBufferSize + 1);
EXPECT_TRUE(ptr == NULL); EXPECT_TRUE(ptr == NULL);
...@@ -214,7 +218,7 @@ TEST_F(TransferBufferTest, TooLargeAllocation) { ...@@ -214,7 +218,7 @@ TEST_F(TransferBufferTest, TooLargeAllocation) {
} }
TEST_F(TransferBufferTest, MemoryAlignmentAfterZeroAllocation) { TEST_F(TransferBufferTest, MemoryAlignmentAfterZeroAllocation) {
Initialize(); Initialize(32u);
void* ptr = transfer_buffer_->Alloc(0); void* ptr = transfer_buffer_->Alloc(0);
EXPECT_EQ((reinterpret_cast<uintptr_t>(ptr) & (kAlignment - 1)), 0u); EXPECT_EQ((reinterpret_cast<uintptr_t>(ptr) & (kAlignment - 1)), 0u);
transfer_buffer_->FreePendingToken(ptr, helper_->InsertToken()); transfer_buffer_->FreePendingToken(ptr, helper_->InsertToken());
...@@ -224,6 +228,32 @@ TEST_F(TransferBufferTest, MemoryAlignmentAfterZeroAllocation) { ...@@ -224,6 +228,32 @@ TEST_F(TransferBufferTest, MemoryAlignmentAfterZeroAllocation) {
transfer_buffer_->FreePendingToken(ptr, helper_->InsertToken()); transfer_buffer_->FreePendingToken(ptr, helper_->InsertToken());
} }
TEST_F(TransferBufferTest, Flush) {
Initialize(16u);
unsigned int size_allocated = 0;
for (int i = 0; i < 8; ++i) {
void* ptr = transfer_buffer_->AllocUpTo(8u, &size_allocated);
ASSERT_TRUE(ptr != NULL);
EXPECT_EQ(8u, size_allocated);
if (i % 2) {
EXPECT_CALL(*command_buffer(), Flush(_))
.Times(1)
.RetiresOnSaturation();
}
transfer_buffer_->FreePendingToken(ptr, helper_->InsertToken());
}
for (int i = 0; i < 8; ++i) {
void* ptr = transfer_buffer_->Alloc(8u);
ASSERT_TRUE(ptr != NULL);
if (i % 2) {
EXPECT_CALL(*command_buffer(), Flush(_))
.Times(1)
.RetiresOnSaturation();
}
transfer_buffer_->FreePendingToken(ptr, helper_->InsertToken());
}
}
class MockClientCommandBufferCanFail : public MockClientCommandBufferMockFlush { class MockClientCommandBufferCanFail : public MockClientCommandBufferMockFlush {
public: public:
MockClientCommandBufferCanFail() = default; MockClientCommandBufferCanFail() = default;
...@@ -291,8 +321,12 @@ void TransferBufferExpandContractTest::SetUp() { ...@@ -291,8 +321,12 @@ void TransferBufferExpandContractTest::SetUp() {
transfer_buffer_.reset(new TransferBuffer(helper_.get())); transfer_buffer_.reset(new TransferBuffer(helper_.get()));
ASSERT_TRUE(transfer_buffer_->Initialize( ASSERT_TRUE(transfer_buffer_->Initialize(
kStartTransferBufferSize, kStartingOffset, kMinTransferBufferSize, kStartTransferBufferSize,
kMaxTransferBufferSize, kAlignment)); kStartingOffset,
kMinTransferBufferSize,
kMaxTransferBufferSize,
kAlignment,
0));
} }
void TransferBufferExpandContractTest::TearDown() { void TransferBufferExpandContractTest::TearDown() {
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment