Skip to content

Commit

Permalink
Vulkan: Add new threshold for CPU buffer subdata
Browse files Browse the repository at this point in the history
  Currently, with preferCPUForBufferSubData enabled, when the GPU is
busy, we use the CPU to copy subdata, which includes duplicating the
buffer. If the buffer is large, this can cause performance regression.

To avoid this, we should avoid using the CPU in cases that can lead to
such regressions.

* Enabled preferCPUForBufferSubData for all ARM devices.

* This feature is only used in the following cases:
  * If the buffer is smaller than than 32K; or
  * If the copy size is greater than 1/8 of the total buffer size.

* Significant performance improvement observed in several traces.

Bug: b/360118138
Change-Id: Ibc8e3de9b4e081c69457c85facba893283c8fb38
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/5824347
Reviewed-by: Charlie Lao <[email protected]>
Reviewed-by: Shahbaz Youssefi <[email protected]>
Commit-Queue: Amirali Abdolrashidi <[email protected]>
  • Loading branch information
aabdolrashidi authored and Angle LUCI CQ committed Sep 10, 2024
1 parent 53476d6 commit 941b3df
Show file tree
Hide file tree
Showing 2 changed files with 35 additions and 5 deletions.
38 changes: 34 additions & 4 deletions src/libANGLE/renderer/vulkan/BufferVk.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -135,10 +135,40 @@ VkMemoryPropertyFlags GetStorageMemoryType(vk::Renderer *renderer,

bool ShouldAllocateNewMemoryForUpdate(ContextVk *contextVk, size_t subDataSize, size_t bufferSize)
{
// A sub data update with size > 50% of buffer size meets the threshold
// to acquire a new BufferHelper from the pool.
return contextVk->getRenderer()->getFeatures().preferCPUForBufferSubData.enabled ||
subDataSize > (bufferSize / 2);
// A sub-data update with size > 50% of buffer size meets the threshold to acquire a new
// BufferHelper from the pool.
size_t halfBufferSize = bufferSize / 2;
if (subDataSize > halfBufferSize)
{
return true;
}

// If the GPU is busy, it is possible to use the CPU for updating sub-data instead, but since it
// would need to create a duplicate of the buffer, a large enough buffer copy could result in a
// performance regression.
if (contextVk->getFeatures().preferCPUForBufferSubData.enabled)
{
// If the buffer is small enough, the cost of barrier associated with the GPU copy likely
// exceeds the overhead with the CPU copy. Duplicating the buffer allows the CPU to write to
// the buffer immediately, thus avoiding the barrier that prevents parallel operation.
constexpr size_t kCpuCopyBufferSizeThreshold = 32 * 1024;
if (bufferSize < kCpuCopyBufferSizeThreshold)
{
return true;
}

// To use CPU for the sub-data update in larger buffers, the update should be sizable enough
// compared to the whole buffer size. The threshold is chosen based on perf data collected
// from Pixel devices. At 1/8 of buffer size, the CPU overhead associated with extra data
// copy weighs less than serialization caused by barriers.
size_t subDataThreshold = bufferSize / 8;
if (subDataSize > subDataThreshold)
{
return true;
}
}

return false;
}

bool ShouldUseCPUToCopyData(ContextVk *contextVk,
Expand Down
2 changes: 1 addition & 1 deletion src/libANGLE/renderer/vulkan/vk_renderer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4659,7 +4659,7 @@ void Renderer::initFeatures(const vk::ExtensionNameList &deviceExtensionNames,
// ARM does buffer copy on geometry pipeline, which may create a GPU pipeline bubble that
// prevents vertex shader to overlap with fragment shader on job manager based architecture. For
// now we always choose CPU to do copy on ARM job manager based GPU.
ANGLE_FEATURE_CONDITION(&mFeatures, preferCPUForBufferSubData, isMaliJobManagerBasedGPU);
ANGLE_FEATURE_CONDITION(&mFeatures, preferCPUForBufferSubData, isARM);

// On android, we usually are GPU limited, we try to use CPU to do data copy when other
// conditions are the same. Set to zero will use GPU to do copy. This is subject to further
Expand Down

0 comments on commit 941b3df

Please sign in to comment.