Commit 295d2ccd by Shahbaz Youssefi Committed by Commit Bot

Vulkan: Generate perf warnings on suboptimal paths

Using KHR_debug features, this change creates a performance-warning-generation macro and employs it in a handful of locations to provide useful feedback to application developers. The warnings added in this change are not exhaustive. Bug: angleproject:3461 Bug: angleproject:4900 Change-Id: Id62435d170d90c5be9c1c5cab2d6779ccb58345e Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/2372628Reviewed-by: 's avatarGeoff Lang <geofflang@chromium.org> Reviewed-by: 's avatarJamie Madill <jmadill@chromium.org> Commit-Queue: Shahbaz Youssefi <syoussefi@chromium.org>
parent d19c08c9
...@@ -3689,6 +3689,8 @@ void Context::blitFramebuffer(GLint srcX0, ...@@ -3689,6 +3689,8 @@ void Context::blitFramebuffer(GLint srcX0,
// Early out if none of the specified attachments exist or are enabled. // Early out if none of the specified attachments exist or are enabled.
if (mask == 0) if (mask == 0)
{ {
ANGLE_PERF_WARNING(mState.getDebug(), GL_DEBUG_SEVERITY_LOW,
"BlitFramebuffer called for non-existing buffers");
return; return;
} }
...@@ -3735,6 +3737,8 @@ void Context::clear(GLbitfield mask) ...@@ -3735,6 +3737,8 @@ void Context::clear(GLbitfield mask)
if (mask == 0) if (mask == 0)
{ {
ANGLE_PERF_WARNING(mState.getDebug(), GL_DEBUG_SEVERITY_LOW,
"Clear called for non-existing buffers");
return; return;
} }
......
...@@ -338,6 +338,24 @@ size_t Debug::getGroupStackDepth() const ...@@ -338,6 +338,24 @@ size_t Debug::getGroupStackDepth() const
return mGroups.size(); return mGroups.size();
} }
void Debug::insertPerfWarning(GLenum severity, const char *message, uint32_t *repeatCount) const
{
constexpr uint32_t kMaxRepeat = 4;
if (*repeatCount >= kMaxRepeat)
{
return;
}
++*repeatCount;
std::string msg = message;
if (*repeatCount == kMaxRepeat)
{
msg += " (this message will no longer repeat)";
}
insertMessage(GL_DEBUG_SOURCE_API, GL_DEBUG_TYPE_PERFORMANCE, 0, severity, std::move(msg),
gl::LOG_INFO);
}
bool Debug::isMessageEnabled(GLenum source, GLenum type, GLuint id, GLenum severity) const bool Debug::isMessageEnabled(GLenum source, GLenum type, GLuint id, GLenum severity) const
{ {
if (!mOutputEnabled) if (!mOutputEnabled)
......
...@@ -81,6 +81,9 @@ class Debug : angle::NonCopyable ...@@ -81,6 +81,9 @@ class Debug : angle::NonCopyable
void popGroup(); void popGroup();
size_t getGroupStackDepth() const; size_t getGroupStackDepth() const;
// Helper for ANGLE_PERF_WARNING
void insertPerfWarning(GLenum severity, const char *message, uint32_t *repeatCount) const;
private: private:
bool isMessageEnabled(GLenum source, GLenum type, GLuint id, GLenum severity) const; bool isMessageEnabled(GLenum source, GLenum type, GLuint id, GLenum severity) const;
...@@ -162,4 +165,13 @@ class Debug : angle::NonCopyable ...@@ -162,4 +165,13 @@ class Debug : angle::NonCopyable
angle::PackedEnumBitSet<MessageType> mEnabledMessageTypes; angle::PackedEnumBitSet<MessageType> mEnabledMessageTypes;
}; };
} // namespace egl } // namespace egl
// Generate a perf warning. Only outputs the same message a few times to avoid spamming the logs.
#define ANGLE_PERF_WARNING(debug, severity, message) \
do \
{ \
static uint32_t sRepeatCount = 0; \
(debug).insertPerfWarning(severity, message, &sRepeatCount); \
} while (0)
#endif // LIBANGLE_DEBUG_H_ #endif // LIBANGLE_DEBUG_H_
...@@ -157,6 +157,7 @@ angle::Result BufferVk::initializeShadowBuffer(ContextVk *contextVk, ...@@ -157,6 +157,7 @@ angle::Result BufferVk::initializeShadowBuffer(ContextVk *contextVk,
// For now, enable shadow buffers only for pixel unpack buffers. // For now, enable shadow buffers only for pixel unpack buffers.
// If usecases present themselves, we can enable them for other buffer types. // If usecases present themselves, we can enable them for other buffer types.
// Note: If changed, update the waitForIdle message in BufferVk::copySubData to reflect it.
if (target == gl::BufferBinding::PixelUnpack) if (target == gl::BufferBinding::PixelUnpack)
{ {
// Initialize the shadow buffer // Initialize the shadow buffer
...@@ -262,7 +263,9 @@ angle::Result BufferVk::copySubData(const gl::Context *context, ...@@ -262,7 +263,9 @@ angle::Result BufferVk::copySubData(const gl::Context *context,
// all recorded and in-flight commands involving the source buffer. // all recorded and in-flight commands involving the source buffer.
if (mShadowBuffer.valid()) if (mShadowBuffer.valid())
{ {
ANGLE_TRY(sourceBuffer.waitForIdle(contextVk)); ANGLE_TRY(sourceBuffer.waitForIdle(
contextVk,
"GPU stall due to copy from buffer in use by the GPU to a pixel unpack buffer"));
// Update the shadow buffer // Update the shadow buffer
uint8_t *srcPtr; uint8_t *srcPtr;
...@@ -334,7 +337,8 @@ angle::Result BufferVk::mapRangeImpl(ContextVk *contextVk, ...@@ -334,7 +337,8 @@ angle::Result BufferVk::mapRangeImpl(ContextVk *contextVk,
if ((access & GL_MAP_UNSYNCHRONIZED_BIT) == 0) if ((access & GL_MAP_UNSYNCHRONIZED_BIT) == 0)
{ {
ANGLE_TRY(mBuffer->waitForIdle(contextVk)); ANGLE_TRY(mBuffer->waitForIdle(contextVk,
"GPU stall due to mapping buffer in use by the GPU"));
} }
ANGLE_TRY(mBuffer->mapWithOffset(contextVk, reinterpret_cast<uint8_t **>(mapPtr), ANGLE_TRY(mBuffer->mapWithOffset(contextVk, reinterpret_cast<uint8_t **>(mapPtr),
...@@ -406,7 +410,9 @@ angle::Result BufferVk::getSubData(const gl::Context *context, ...@@ -406,7 +410,9 @@ angle::Result BufferVk::getSubData(const gl::Context *context,
{ {
ASSERT(mBuffer && mBuffer->valid()); ASSERT(mBuffer && mBuffer->valid());
ContextVk *contextVk = vk::GetImpl(context); ContextVk *contextVk = vk::GetImpl(context);
ANGLE_TRY(mBuffer->waitForIdle(contextVk)); // Note: This function is used for ANGLE's capture/replay tool, so no performance warnings
// is generated.
ANGLE_TRY(mBuffer->waitForIdle(contextVk, nullptr));
if (mBuffer->isMapped()) if (mBuffer->isMapped())
{ {
memcpy(outData, mBuffer->getMappedMemory() + offset, size); memcpy(outData, mBuffer->getMappedMemory() + offset, size);
...@@ -451,6 +457,9 @@ angle::Result BufferVk::getIndexRange(const gl::Context *context, ...@@ -451,6 +457,9 @@ angle::Result BufferVk::getIndexRange(const gl::Context *context,
if (!mShadowBuffer.valid()) if (!mShadowBuffer.valid())
{ {
ANGLE_PERF_WARNING(contextVk->getDebug(), GL_DEBUG_SEVERITY_HIGH,
"GPU stall due to index range validation");
// Needed before reading buffer or we could get stale data. // Needed before reading buffer or we could get stale data.
ANGLE_TRY(mBuffer->finishRunningCommands(contextVk)); ANGLE_TRY(mBuffer->finishRunningCommands(contextVk));
......
...@@ -1100,6 +1100,10 @@ angle::Result ContextVk::setupIndexedDraw(const gl::Context *context, ...@@ -1100,6 +1100,10 @@ angle::Result ContextVk::setupIndexedDraw(const gl::Context *context,
} }
if (shouldConvertUint8VkIndexType(indexType) && mGraphicsDirtyBits[DIRTY_BIT_INDEX_BUFFER]) if (shouldConvertUint8VkIndexType(indexType) && mGraphicsDirtyBits[DIRTY_BIT_INDEX_BUFFER])
{ {
ANGLE_PERF_WARNING(getDebug(), GL_DEBUG_SEVERITY_LOW,
"Potential inefficiency emulating uint8 vertex attributes due to "
"lack of hardware support");
BufferVk *bufferVk = vk::GetImpl(elementArrayBuffer); BufferVk *bufferVk = vk::GetImpl(elementArrayBuffer);
vk::BufferHelper &bufferHelper = bufferVk->getBuffer(); vk::BufferHelper &bufferHelper = bufferVk->getBuffer();
...@@ -2347,6 +2351,10 @@ angle::Result ContextVk::drawElementsIndirect(const gl::Context *context, ...@@ -2347,6 +2351,10 @@ angle::Result ContextVk::drawElementsIndirect(const gl::Context *context,
if (shouldConvertUint8VkIndexType(type) && mGraphicsDirtyBits[DIRTY_BIT_INDEX_BUFFER]) if (shouldConvertUint8VkIndexType(type) && mGraphicsDirtyBits[DIRTY_BIT_INDEX_BUFFER])
{ {
ANGLE_PERF_WARNING(getDebug(), GL_DEBUG_SEVERITY_LOW,
"Potential inefficiency emulating uint8 vertex attributes due to lack "
"of hardware support");
vk::BufferHelper *dstIndirectBuf; vk::BufferHelper *dstIndirectBuf;
VkDeviceSize dstIndirectBufOffset; VkDeviceSize dstIndirectBufOffset;
......
...@@ -477,6 +477,7 @@ class ContextVk : public ContextImpl, public vk::Context ...@@ -477,6 +477,7 @@ class ContextVk : public ContextImpl, public vk::Context
bool useOldRewriteStructSamplers() const { return mUseOldRewriteStructSamplers; } bool useOldRewriteStructSamplers() const { return mUseOldRewriteStructSamplers; }
const gl::Debug &getDebug() const { return mState.getDebug(); }
const gl::OverlayType *getOverlay() const { return mState.getOverlay(); } const gl::OverlayType *getOverlay() const { return mState.getOverlay(); }
vk::ResourceUseList &getResourceUseList() { return mResourceUseList; } vk::ResourceUseList &getResourceUseList() { return mResourceUseList; }
......
...@@ -345,6 +345,12 @@ angle::Result FramebufferVk::invalidateSub(const gl::Context *context, ...@@ -345,6 +345,12 @@ angle::Result FramebufferVk::invalidateSub(const gl::Context *context,
// the invalidated area). // the invalidated area).
ANGLE_TRY(invalidateImpl(contextVk, count, attachments, true)); ANGLE_TRY(invalidateImpl(contextVk, count, attachments, true));
} }
else
{
ANGLE_PERF_WARNING(
contextVk->getDebug(), GL_DEBUG_SEVERITY_LOW,
"InvalidateSubFramebuffer ignored due to area not covering the render area");
}
return angle::Result::Continue; return angle::Result::Continue;
} }
...@@ -1189,6 +1195,9 @@ angle::Result FramebufferVk::blit(const gl::Context *context, ...@@ -1189,6 +1195,9 @@ angle::Result FramebufferVk::blit(const gl::Context *context,
// If shader stencil export is not present, blit stencil through a different path. // If shader stencil export is not present, blit stencil through a different path.
if (blitStencilBuffer && !hasShaderStencilExport) if (blitStencilBuffer && !hasShaderStencilExport)
{ {
ANGLE_PERF_WARNING(contextVk->getDebug(), GL_DEBUG_SEVERITY_LOW,
"Inefficient BlitFramebuffer operation on the stencil aspect "
"due to lack of shader stencil export support");
ANGLE_TRY(utilsVk.stencilBlitResolveNoShaderExport( ANGLE_TRY(utilsVk.stencilBlitResolveNoShaderExport(
contextVk, this, depthStencilImage, &stencilView.get(), params)); contextVk, this, depthStencilImage, &stencilView.get(), params));
} }
......
...@@ -198,6 +198,8 @@ angle::Result QueryVk::getResult(const gl::Context *context, bool wait) ...@@ -198,6 +198,8 @@ angle::Result QueryVk::getResult(const gl::Context *context, bool wait)
{ {
return angle::Result::Continue; return angle::Result::Continue;
} }
ANGLE_PERF_WARNING(contextVk->getDebug(), GL_DEBUG_SEVERITY_HIGH,
"GPU stall due to waiting on uncompleted query");
ANGLE_TRY(contextVk->finishToSerial(mQueryHelper.getStoredQueueSerial())); ANGLE_TRY(contextVk->finishToSerial(mQueryHelper.getStoredQueueSerial()));
} }
......
...@@ -36,7 +36,7 @@ angle::Result Resource::finishRunningCommands(ContextVk *contextVk) ...@@ -36,7 +36,7 @@ angle::Result Resource::finishRunningCommands(ContextVk *contextVk)
return contextVk->finishToSerial(mUse.getSerial()); return contextVk->finishToSerial(mUse.getSerial());
} }
angle::Result Resource::waitForIdle(ContextVk *contextVk) angle::Result Resource::waitForIdle(ContextVk *contextVk, const char *debugMessage)
{ {
// If there are pending commands for the resource, flush them. // If there are pending commands for the resource, flush them.
if (usedInRecordedCommands()) if (usedInRecordedCommands())
...@@ -47,6 +47,10 @@ angle::Result Resource::waitForIdle(ContextVk *contextVk) ...@@ -47,6 +47,10 @@ angle::Result Resource::waitForIdle(ContextVk *contextVk)
// Make sure the driver is done with the resource. // Make sure the driver is done with the resource.
if (usedInRunningCommands(contextVk->getLastCompletedQueueSerial())) if (usedInRunningCommands(contextVk->getLastCompletedQueueSerial()))
{ {
if (debugMessage)
{
ANGLE_PERF_WARNING(contextVk->getDebug(), GL_DEBUG_SEVERITY_HIGH, debugMessage);
}
ANGLE_TRY(finishRunningCommands(contextVk)); ANGLE_TRY(finishRunningCommands(contextVk));
} }
......
...@@ -180,7 +180,7 @@ class Resource : angle::NonCopyable ...@@ -180,7 +180,7 @@ class Resource : angle::NonCopyable
angle::Result finishRunningCommands(ContextVk *contextVk); angle::Result finishRunningCommands(ContextVk *contextVk);
// Complete all recorded and in-flight commands involving this resource // Complete all recorded and in-flight commands involving this resource
angle::Result waitForIdle(ContextVk *contextVk); angle::Result waitForIdle(ContextVk *contextVk, const char *debugMessage);
// Adds the resource to a resource use list. // Adds the resource to a resource use list.
void retain(ResourceUseList *resourceUseList); void retain(ResourceUseList *resourceUseList);
......
...@@ -618,6 +618,9 @@ angle::Result TextureVk::copySubImageImpl(const gl::Context *context, ...@@ -618,6 +618,9 @@ angle::Result TextureVk::copySubImageImpl(const gl::Context *context,
contextVk->getRotationReadFramebuffer()); contextVk->getRotationReadFramebuffer());
} }
ANGLE_PERF_WARNING(contextVk->getDebug(), GL_DEBUG_SEVERITY_HIGH,
"Texture copied on CPU due to format restrictions");
// Do a CPU readback that does the conversion, and then stage the change to the pixel buffer. // Do a CPU readback that does the conversion, and then stage the change to the pixel buffer.
ANGLE_TRY(mImage->stageSubresourceUpdateFromFramebuffer( ANGLE_TRY(mImage->stageSubresourceUpdateFromFramebuffer(
context, offsetImageIndex, clippedSourceArea, modifiedDestOffset, context, offsetImageIndex, clippedSourceArea, modifiedDestOffset,
...@@ -670,6 +673,9 @@ angle::Result TextureVk::copySubTextureImpl(ContextVk *contextVk, ...@@ -670,6 +673,9 @@ angle::Result TextureVk::copySubTextureImpl(ContextVk *contextVk,
&source->getCopyImageViewAndRecordUse(contextVk), SurfaceRotation::Identity); &source->getCopyImageViewAndRecordUse(contextVk), SurfaceRotation::Identity);
} }
ANGLE_PERF_WARNING(contextVk->getDebug(), GL_DEBUG_SEVERITY_HIGH,
"Texture copied on CPU due to format restrictions");
if (sourceLevelGL != 0) if (sourceLevelGL != 0)
{ {
WARN() << "glCopyTextureCHROMIUM with sourceLevel != 0 not implemented."; WARN() << "glCopyTextureCHROMIUM with sourceLevel != 0 not implemented.";
...@@ -1602,6 +1608,9 @@ angle::Result TextureVk::generateMipmap(const gl::Context *context) ...@@ -1602,6 +1608,9 @@ angle::Result TextureVk::generateMipmap(const gl::Context *context)
return mImage->generateMipmapsWithBlit(contextVk, maxLevel); return mImage->generateMipmapsWithBlit(contextVk, maxLevel);
} }
ANGLE_PERF_WARNING(contextVk->getDebug(), GL_DEBUG_SEVERITY_HIGH,
"Mipmap generated on CPU due to format restrictions");
// If not possible to generate mipmaps on the GPU, do it on the CPU for conformance. // If not possible to generate mipmaps on the GPU, do it on the CPU for conformance.
return generateMipmapsWithCPU(context); return generateMipmapsWithCPU(context);
} }
......
...@@ -4432,14 +4432,16 @@ angle::Result ImageHelper::flushStagedUpdates(ContextVk *contextVk, ...@@ -4432,14 +4432,16 @@ angle::Result ImageHelper::flushStagedUpdates(ContextVk *contextVk,
removeSupersededUpdates(skipLevelsMask); removeSupersededUpdates(skipLevelsMask);
// If a clear is requested and we know it just has been cleared with the same value, we drop the // If a clear is requested and we know it has just been cleared with the same value, we drop the
// clear // clear.
if (mSubresourceUpdates.size() == 1) if (mSubresourceUpdates.size() == 1)
{ {
SubresourceUpdate &update = mSubresourceUpdates[0]; SubresourceUpdate &update = mSubresourceUpdates[0];
if (update.updateSource == UpdateSource::Clear && mCurrentSingleClearValue.valid() && if (update.updateSource == UpdateSource::Clear && mCurrentSingleClearValue.valid() &&
mCurrentSingleClearValue.value() == update.clear) mCurrentSingleClearValue.value() == update.clear)
{ {
ANGLE_PERF_WARNING(contextVk->getDebug(), GL_DEBUG_SEVERITY_LOW,
"Repeated Clear on framebuffer attachment dropped");
update.release(contextVk->getRenderer()); update.release(contextVk->getRenderer());
mSubresourceUpdates.clear(); mSubresourceUpdates.clear();
return angle::Result::Continue; return angle::Result::Continue;
......
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