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

GPU: Fix transfer buffers not being freed promptly

A previous change removed the command buffer flush when destroying a
transfer buffer:
https://chromium-review.googlesource.com/c/chromium/src/+/1105466

If we want to free a transfer buffer promptly, we now need to flush
separately, and we do. Unfortunately if no commands were issued between
the call to DestroyTransferBuffer and the flush, the flush might not
execute because the CommandBufferProxy would pass an outdated flush ID
to GpuChannelHost::EnsureFlush. The fix is to inform the
CommandBufferProxy about the new flush ID when it calls
DestroyTransferBuffer.

This should fix the memory regressions on the perf waterfall noted in
bug 855402.

Bug: 855402, 850271, 835353, 828363
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: Id125c5124b26763e692c1fe8067df28761fa6dab
Reviewed-on: https://chromium-review.googlesource.com/1121590Reviewed-by: default avatarAntoine Labour <piman@chromium.org>
Commit-Queue: James Darpinian <jdarpinian@chromium.org>
Cr-Commit-Position: refs/heads/master@{#572074}
parent b18c59fe
...@@ -411,7 +411,7 @@ void CommandBufferProxyImpl::DestroyTransferBuffer(int32_t id) { ...@@ -411,7 +411,7 @@ void CommandBufferProxyImpl::DestroyTransferBuffer(int32_t id) {
if (last_state_.error != gpu::error::kNoError) if (last_state_.error != gpu::error::kNoError)
return; return;
channel_->DestroyTransferBuffer(route_id_, id); last_flush_id_ = channel_->DestroyTransferBuffer(route_id_, id);
} }
void CommandBufferProxyImpl::SetGpuControlClient(GpuControlClient* client) { void CommandBufferProxyImpl::SetGpuControlClient(GpuControlClient* client) {
......
...@@ -107,8 +107,8 @@ uint32_t GpuChannelHost::OrderingBarrier( ...@@ -107,8 +107,8 @@ uint32_t GpuChannelHost::OrderingBarrier(
return flush_params.flush_id; return flush_params.flush_id;
} }
void GpuChannelHost::DestroyTransferBuffer(int32_t route_id, uint32_t GpuChannelHost::DestroyTransferBuffer(int32_t route_id,
int32_t id_to_destroy) { int32_t id_to_destroy) {
AutoLock lock(context_lock_); AutoLock lock(context_lock_);
flush_list_.push_back(FlushParams()); flush_list_.push_back(FlushParams());
...@@ -117,6 +117,7 @@ void GpuChannelHost::DestroyTransferBuffer(int32_t route_id, ...@@ -117,6 +117,7 @@ void GpuChannelHost::DestroyTransferBuffer(int32_t route_id,
flush_params.route_id = route_id; flush_params.route_id = route_id;
flush_params.put_offset = -1; flush_params.put_offset = -1;
flush_params.transfer_buffer_id_to_destroy = id_to_destroy; flush_params.transfer_buffer_id_to_destroy = id_to_destroy;
return flush_params.flush_id;
} }
void GpuChannelHost::EnsureFlush(uint32_t flush_id) { void GpuChannelHost::EnsureFlush(uint32_t flush_id) {
......
...@@ -122,8 +122,9 @@ class GPU_EXPORT GpuChannelHost ...@@ -122,8 +122,9 @@ class GPU_EXPORT GpuChannelHost
int32_t ReserveTransferBufferId(); int32_t ReserveTransferBufferId();
// An ordering barrier must be placed after any commands that use the buffer // An ordering barrier must be placed after any commands that use the buffer
// before it is safe to call this function to destroy it. // before it is safe to call this function to destroy it. Returns a flush id
void DestroyTransferBuffer(int32_t route_id, int32_t id_to_destroy); // just like OrderingBarrier.
uint32_t DestroyTransferBuffer(int32_t route_id, int32_t id_to_destroy);
// Reserve one unused image ID. // Reserve one unused image ID.
int32_t ReserveImageId(); int32_t ReserveImageId();
......
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