Commit b156a753 by Courtney Goeltzenleuchter Committed by Commit Bot

Move LayoutCaches to ShareGroup

Testing with TSN found a race condition with RefCounted objects (DescriptorSetLayout and PipelineLayout). Rather than add more lock calls to protect accesses to mRefCount and mObject recommendation was to put these caches in the ShareGroup (basically part of the context). Locking at the GL level will ensure that two threads that share the same context will not access the ShareGroup at the same time. The ShareGroup also works because these layouts are not destroyed until the context is destroyed so don't have to worry about other threads (e.g. command processor thread) accessing them. Bug: b/168744561 Change-Id: Icc0aa07bf4787a69572d6ec62da2f21d286232c3 Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/2437509 Commit-Queue: Courtney Goeltzenleuchter <courtneygo@google.com> Reviewed-by: 's avatarJamie Madill <jmadill@chromium.org> Reviewed-by: 's avatarCharlie Lao <cclao@google.com>
parent b3859a3c
...@@ -619,12 +619,14 @@ egl::Error Context::onDestroy(const egl::Display *display) ...@@ -619,12 +619,14 @@ egl::Error Context::onDestroy(const egl::Display *display)
mState.mFramebufferManager->release(this); mState.mFramebufferManager->release(this);
mState.mMemoryObjectManager->release(this); mState.mMemoryObjectManager->release(this);
mState.mSemaphoreManager->release(this); mState.mSemaphoreManager->release(this);
mState.mShareGroup->release(this);
mThreadPool.reset(); mThreadPool.reset();
mImplementation->onDestroy(this); mImplementation->onDestroy(this);
// Backend requires implementation to be destroyed first to close down all the objects
mState.mShareGroup->release(display);
mOverlay.destroy(this); mOverlay.destroy(this);
return egl::NoError(); return egl::NoError();
......
...@@ -471,10 +471,14 @@ void ShareGroup::addRef() ...@@ -471,10 +471,14 @@ void ShareGroup::addRef()
mRefCount++; mRefCount++;
} }
void ShareGroup::release(const gl::Context *context) void ShareGroup::release(const Display *display)
{ {
if (--mRefCount == 0) if (--mRefCount == 0)
{ {
if (mImplementation)
{
mImplementation->onDestroy(display);
}
delete this; delete this;
} }
} }
......
...@@ -73,7 +73,7 @@ class ShareGroup final : angle::NonCopyable ...@@ -73,7 +73,7 @@ class ShareGroup final : angle::NonCopyable
void addRef(); void addRef();
void release(const gl::Context *context); void release(const egl::Display *display);
rx::ShareGroupImpl *getImplementation() const { return mImplementation; } rx::ShareGroupImpl *getImplementation() const { return mImplementation; }
......
...@@ -57,6 +57,7 @@ class ShareGroupImpl : angle::NonCopyable ...@@ -57,6 +57,7 @@ class ShareGroupImpl : angle::NonCopyable
public: public:
ShareGroupImpl() {} ShareGroupImpl() {}
virtual ~ShareGroupImpl() {} virtual ~ShareGroupImpl() {}
virtual void onDestroy(const egl::Display *display) {}
}; };
class DisplayImpl : public EGLImplFactory, public angle::Subject class DisplayImpl : public EGLImplFactory, public angle::Subject
......
...@@ -852,7 +852,7 @@ angle::Result ContextVk::initialize() ...@@ -852,7 +852,7 @@ angle::Result ContextVk::initialize()
vk::DescriptorSetLayoutDesc desc = vk::DescriptorSetLayoutDesc desc =
getDriverUniformsDescriptorSetDesc(kPipelineStages[pipeline]); getDriverUniformsDescriptorSetDesc(kPipelineStages[pipeline]);
ANGLE_TRY(mRenderer->getDescriptorSetLayout( ANGLE_TRY(getDescriptorSetLayoutCache().getDescriptorSetLayout(
this, desc, &mDriverUniforms[pipeline].descriptorSetLayout)); this, desc, &mDriverUniforms[pipeline].descriptorSetLayout));
vk::DescriptorSetLayoutBindingVector bindingVector; vk::DescriptorSetLayoutBindingVector bindingVector;
......
...@@ -16,6 +16,7 @@ ...@@ -16,6 +16,7 @@
#include "common/vulkan/vk_headers.h" #include "common/vulkan/vk_headers.h"
#include "libANGLE/renderer/ContextImpl.h" #include "libANGLE/renderer/ContextImpl.h"
#include "libANGLE/renderer/renderer_utils.h" #include "libANGLE/renderer/renderer_utils.h"
#include "libANGLE/renderer/vulkan/DisplayVk.h"
#include "libANGLE/renderer/vulkan/OverlayVk.h" #include "libANGLE/renderer/vulkan/OverlayVk.h"
#include "libANGLE/renderer/vulkan/PersistentCommandPool.h" #include "libANGLE/renderer/vulkan/PersistentCommandPool.h"
#include "libANGLE/renderer/vulkan/RendererVk.h" #include "libANGLE/renderer/vulkan/RendererVk.h"
...@@ -230,6 +231,17 @@ class ContextVk : public ContextImpl, public vk::Context ...@@ -230,6 +231,17 @@ class ContextVk : public ContextImpl, public vk::Context
const GLuint *baseInstances, const GLuint *baseInstances,
GLsizei drawcount) override; GLsizei drawcount) override;
// ShareGroup
ShareGroupVk *getShareGroupVk() { return mShareGroupVk; }
PipelineLayoutCache &getPipelineLayoutCache()
{
return mShareGroupVk->getPipelineLayoutCache();
}
DescriptorSetLayoutCache &getDescriptorSetLayoutCache()
{
return mShareGroupVk->getDescriptorSetLayoutCache();
}
// Device loss // Device loss
gl::GraphicsResetStatus getResetStatus() override; gl::GraphicsResetStatus getResetStatus() override;
......
...@@ -289,4 +289,12 @@ bool DisplayVk::isRobustResourceInitEnabled() const ...@@ -289,4 +289,12 @@ bool DisplayVk::isRobustResourceInitEnabled() const
// We return true if any surface was created with robust resource init enabled. // We return true if any surface was created with robust resource init enabled.
return mHasSurfaceWithRobustInit; return mHasSurfaceWithRobustInit;
} }
void ShareGroupVk::onDestroy(const egl::Display *display)
{
DisplayVk *displayVk = vk::GetImpl(display);
mPipelineLayoutCache.destroy(displayVk->getDevice());
mDescriptorSetLayoutCache.destroy(displayVk->getDevice());
}
} // namespace rx } // namespace rx
...@@ -12,6 +12,7 @@ ...@@ -12,6 +12,7 @@
#include "common/MemoryBuffer.h" #include "common/MemoryBuffer.h"
#include "libANGLE/renderer/DisplayImpl.h" #include "libANGLE/renderer/DisplayImpl.h"
#include "libANGLE/renderer/vulkan/vk_cache_utils.h"
#include "libANGLE/renderer/vulkan/vk_utils.h" #include "libANGLE/renderer/vulkan/vk_utils.h"
namespace rx namespace rx
...@@ -22,6 +23,20 @@ class ShareGroupVk : public ShareGroupImpl ...@@ -22,6 +23,20 @@ class ShareGroupVk : public ShareGroupImpl
{ {
public: public:
ShareGroupVk() {} ShareGroupVk() {}
void onDestroy(const egl::Display *display) override;
// PipelineLayoutCache and DescriptorSetLayoutCache can be shared between multiple threads
// accessing them via shared contexts. The ShareGroup locks around gl entrypoints ensuring
// synchronous update to the caches.
PipelineLayoutCache &getPipelineLayoutCache() { return mPipelineLayoutCache; }
DescriptorSetLayoutCache &getDescriptorSetLayoutCache() { return mDescriptorSetLayoutCache; }
private:
// ANGLE uses a PipelineLayout cache to store compatible pipeline layouts.
PipelineLayoutCache mPipelineLayoutCache;
// DescriptorSetLayouts are also managed in a cache.
DescriptorSetLayoutCache mDescriptorSetLayoutCache;
}; };
class DisplayVk : public DisplayImpl, public vk::Context class DisplayVk : public DisplayImpl, public vk::Context
......
...@@ -10,6 +10,7 @@ ...@@ -10,6 +10,7 @@
#include "libANGLE/renderer/glslang_wrapper_utils.h" #include "libANGLE/renderer/glslang_wrapper_utils.h"
#include "libANGLE/renderer/vulkan/BufferVk.h" #include "libANGLE/renderer/vulkan/BufferVk.h"
#include "libANGLE/renderer/vulkan/DisplayVk.h"
#include "libANGLE/renderer/vulkan/GlslangWrapperVk.h" #include "libANGLE/renderer/vulkan/GlslangWrapperVk.h"
#include "libANGLE/renderer/vulkan/ProgramPipelineVk.h" #include "libANGLE/renderer/vulkan/ProgramPipelineVk.h"
#include "libANGLE/renderer/vulkan/ProgramVk.h" #include "libANGLE/renderer/vulkan/ProgramVk.h"
...@@ -736,7 +737,6 @@ angle::Result ProgramExecutableVk::createPipelineLayout( ...@@ -736,7 +737,6 @@ angle::Result ProgramExecutableVk::createPipelineLayout(
{ {
const gl::State &glState = glContext->getState(); const gl::State &glState = glContext->getState();
ContextVk *contextVk = vk::GetImpl(glContext); ContextVk *contextVk = vk::GetImpl(glContext);
RendererVk *renderer = contextVk->getRenderer();
gl::TransformFeedback *transformFeedback = glState.getCurrentTransformFeedback(); gl::TransformFeedback *transformFeedback = glState.getCurrentTransformFeedback();
const gl::ProgramExecutable &glExecutable = getGlExecutable(); const gl::ProgramExecutable &glExecutable = getGlExecutable();
const gl::ShaderBitSet &linkedShaderStages = glExecutable.getLinkedShaderStages(); const gl::ShaderBitSet &linkedShaderStages = glExecutable.getLinkedShaderStages();
...@@ -786,7 +786,7 @@ angle::Result ProgramExecutableVk::createPipelineLayout( ...@@ -786,7 +786,7 @@ angle::Result ProgramExecutableVk::createPipelineLayout(
xfbBufferCount, &uniformsAndXfbSetDesc); xfbBufferCount, &uniformsAndXfbSetDesc);
} }
ANGLE_TRY(renderer->getDescriptorSetLayout( ANGLE_TRY(contextVk->getDescriptorSetLayoutCache().getDescriptorSetLayout(
contextVk, uniformsAndXfbSetDesc, contextVk, uniformsAndXfbSetDesc,
&mDescriptorSetLayouts[ToUnderlying(DescriptorSetIndex::UniformsAndXfb)])); &mDescriptorSetLayouts[ToUnderlying(DescriptorSetIndex::UniformsAndXfb)]));
...@@ -814,7 +814,7 @@ angle::Result ProgramExecutableVk::createPipelineLayout( ...@@ -814,7 +814,7 @@ angle::Result ProgramExecutableVk::createPipelineLayout(
contextVk->useOldRewriteStructSamplers(), &resourcesSetDesc); contextVk->useOldRewriteStructSamplers(), &resourcesSetDesc);
} }
ANGLE_TRY(renderer->getDescriptorSetLayout( ANGLE_TRY(contextVk->getDescriptorSetLayoutCache().getDescriptorSetLayout(
contextVk, resourcesSetDesc, contextVk, resourcesSetDesc,
&mDescriptorSetLayouts[ToUnderlying(DescriptorSetIndex::ShaderResource)])); &mDescriptorSetLayouts[ToUnderlying(DescriptorSetIndex::ShaderResource)]));
...@@ -829,7 +829,7 @@ angle::Result ProgramExecutableVk::createPipelineLayout( ...@@ -829,7 +829,7 @@ angle::Result ProgramExecutableVk::createPipelineLayout(
activeTextures, &texturesSetDesc); activeTextures, &texturesSetDesc);
} }
ANGLE_TRY(renderer->getDescriptorSetLayout( ANGLE_TRY(contextVk->getDescriptorSetLayoutCache().getDescriptorSetLayout(
contextVk, texturesSetDesc, contextVk, texturesSetDesc,
&mDescriptorSetLayouts[ToUnderlying(DescriptorSetIndex::Texture)])); &mDescriptorSetLayouts[ToUnderlying(DescriptorSetIndex::Texture)]));
...@@ -838,7 +838,7 @@ angle::Result ProgramExecutableVk::createPipelineLayout( ...@@ -838,7 +838,7 @@ angle::Result ProgramExecutableVk::createPipelineLayout(
glExecutable.isCompute() ? VK_SHADER_STAGE_COMPUTE_BIT : VK_SHADER_STAGE_ALL_GRAPHICS; glExecutable.isCompute() ? VK_SHADER_STAGE_COMPUTE_BIT : VK_SHADER_STAGE_ALL_GRAPHICS;
vk::DescriptorSetLayoutDesc driverUniformsSetDesc = vk::DescriptorSetLayoutDesc driverUniformsSetDesc =
contextVk->getDriverUniformsDescriptorSetDesc(driverUniformsStages); contextVk->getDriverUniformsDescriptorSetDesc(driverUniformsStages);
ANGLE_TRY(renderer->getDescriptorSetLayout( ANGLE_TRY(contextVk->getDescriptorSetLayoutCache().getDescriptorSetLayout(
contextVk, driverUniformsSetDesc, contextVk, driverUniformsSetDesc,
&mDescriptorSetLayouts[ToUnderlying(DescriptorSetIndex::DriverUniforms)])); &mDescriptorSetLayouts[ToUnderlying(DescriptorSetIndex::DriverUniforms)]));
...@@ -852,8 +852,8 @@ angle::Result ProgramExecutableVk::createPipelineLayout( ...@@ -852,8 +852,8 @@ angle::Result ProgramExecutableVk::createPipelineLayout(
pipelineLayoutDesc.updateDescriptorSetLayout(DescriptorSetIndex::DriverUniforms, pipelineLayoutDesc.updateDescriptorSetLayout(DescriptorSetIndex::DriverUniforms,
driverUniformsSetDesc); driverUniformsSetDesc);
ANGLE_TRY(renderer->getPipelineLayout(contextVk, pipelineLayoutDesc, mDescriptorSetLayouts, ANGLE_TRY(contextVk->getPipelineLayoutCache().getPipelineLayout(
&mPipelineLayout)); contextVk, pipelineLayoutDesc, mDescriptorSetLayouts, &mPipelineLayout));
// Initialize descriptor pools. // Initialize descriptor pools.
ANGLE_TRY(initDynamicDescriptorPools( ANGLE_TRY(initDynamicDescriptorPools(
......
...@@ -535,9 +535,6 @@ void RendererVk::onDestroy() ...@@ -535,9 +535,6 @@ void RendererVk::onDestroy()
mFenceRecycler.destroy(mDevice); mFenceRecycler.destroy(mDevice);
} }
mPipelineLayoutCache.destroy(mDevice);
mDescriptorSetLayoutCache.destroy(mDevice);
mPipelineCache.destroy(mDevice); mPipelineCache.destroy(mDevice);
mSamplerCache.destroy(this); mSamplerCache.destroy(this);
mYuvConversionCache.destroy(this); mYuvConversionCache.destroy(this);
...@@ -2045,26 +2042,6 @@ const gl::Limitations &RendererVk::getNativeLimitations() const ...@@ -2045,26 +2042,6 @@ const gl::Limitations &RendererVk::getNativeLimitations() const
return mNativeLimitations; return mNativeLimitations;
} }
angle::Result RendererVk::getDescriptorSetLayout(
ContextVk *context,
const vk::DescriptorSetLayoutDesc &desc,
vk::BindingPointer<vk::DescriptorSetLayout> *descriptorSetLayoutOut)
{
std::lock_guard<decltype(mDescriptorSetLayoutCacheMutex)> lock(mDescriptorSetLayoutCacheMutex);
return mDescriptorSetLayoutCache.getDescriptorSetLayout(context, desc, descriptorSetLayoutOut);
}
angle::Result RendererVk::getPipelineLayout(
vk::Context *context,
const vk::PipelineLayoutDesc &desc,
const vk::DescriptorSetLayoutPointerArray &descriptorSetLayouts,
vk::BindingPointer<vk::PipelineLayout> *pipelineLayoutOut)
{
std::lock_guard<decltype(mPipelineLayoutCacheMutex)> lock(mPipelineLayoutCacheMutex);
return mPipelineLayoutCache.getPipelineLayout(context, desc, descriptorSetLayouts,
pipelineLayoutOut);
}
angle::Result RendererVk::getPipelineCacheSize(DisplayVk *displayVk, size_t *pipelineCacheSizeOut) angle::Result RendererVk::getPipelineCacheSize(DisplayVk *displayVk, size_t *pipelineCacheSizeOut)
{ {
VkResult result = mPipelineCache.getCacheData(mDevice, pipelineCacheSizeOut, nullptr); VkResult result = mPipelineCache.getCacheData(mDevice, pipelineCacheSizeOut, nullptr);
......
...@@ -134,18 +134,6 @@ class RendererVk : angle::NonCopyable ...@@ -134,18 +134,6 @@ class RendererVk : angle::NonCopyable
const vk::Format &getFormat(angle::FormatID formatID) const { return mFormatTable[formatID]; } const vk::Format &getFormat(angle::FormatID formatID) const { return mFormatTable[formatID]; }
// Queries the descriptor set layout cache. Creates the layout if not present.
angle::Result getDescriptorSetLayout(
ContextVk *context,
const vk::DescriptorSetLayoutDesc &desc,
vk::BindingPointer<vk::DescriptorSetLayout> *descriptorSetLayoutOut);
// Queries the pipeline layout cache. Creates the layout if not present.
angle::Result getPipelineLayout(vk::Context *context,
const vk::PipelineLayoutDesc &desc,
const vk::DescriptorSetLayoutPointerArray &descriptorSetLayouts,
vk::BindingPointer<vk::PipelineLayout> *pipelineLayoutOut);
angle::Result getPipelineCacheSize(DisplayVk *displayVk, size_t *pipelineCacheSizeOut); angle::Result getPipelineCacheSize(DisplayVk *displayVk, size_t *pipelineCacheSizeOut);
angle::Result syncPipelineCacheVk(DisplayVk *displayVk); angle::Result syncPipelineCacheVk(DisplayVk *displayVk);
...@@ -377,14 +365,6 @@ class RendererVk : angle::NonCopyable ...@@ -377,14 +365,6 @@ class RendererVk : angle::NonCopyable
// A cache of VkFormatProperties as queried from the device over time. // A cache of VkFormatProperties as queried from the device over time.
std::array<VkFormatProperties, vk::kNumVkFormats> mFormatProperties; std::array<VkFormatProperties, vk::kNumVkFormats> mFormatProperties;
// ANGLE uses a PipelineLayout cache to store compatible pipeline layouts.
std::mutex mPipelineLayoutCacheMutex;
PipelineLayoutCache mPipelineLayoutCache;
// DescriptorSetLayouts are also managed in a cache.
std::mutex mDescriptorSetLayoutCacheMutex;
DescriptorSetLayoutCache mDescriptorSetLayoutCache;
// Latest validation data for debug overlay. // Latest validation data for debug overlay.
std::string mLastValidationMessage; std::string mLastValidationMessage;
uint32_t mValidationMessageCount; uint32_t mValidationMessageCount;
......
...@@ -10,6 +10,7 @@ ...@@ -10,6 +10,7 @@
#include "libANGLE/renderer/vulkan/UtilsVk.h" #include "libANGLE/renderer/vulkan/UtilsVk.h"
#include "libANGLE/renderer/vulkan/ContextVk.h" #include "libANGLE/renderer/vulkan/ContextVk.h"
#include "libANGLE/renderer/vulkan/DisplayVk.h"
#include "libANGLE/renderer/vulkan/FramebufferVk.h" #include "libANGLE/renderer/vulkan/FramebufferVk.h"
#include "libANGLE/renderer/vulkan/GlslangWrapperVk.h" #include "libANGLE/renderer/vulkan/GlslangWrapperVk.h"
#include "libANGLE/renderer/vulkan/RenderTargetVk.h" #include "libANGLE/renderer/vulkan/RenderTargetVk.h"
...@@ -612,8 +613,6 @@ angle::Result UtilsVk::ensureResourcesInitialized(ContextVk *contextVk, ...@@ -612,8 +613,6 @@ angle::Result UtilsVk::ensureResourcesInitialized(ContextVk *contextVk,
size_t setSizesCount, size_t setSizesCount,
size_t pushConstantsSize) size_t pushConstantsSize)
{ {
RendererVk *renderer = contextVk->getRenderer();
vk::DescriptorSetLayoutDesc descriptorSetDesc; vk::DescriptorSetLayoutDesc descriptorSetDesc;
bool isCompute = function >= Function::ComputeStartIndex; bool isCompute = function >= Function::ComputeStartIndex;
const VkShaderStageFlags descStages = const VkShaderStageFlags descStages =
...@@ -627,7 +626,7 @@ angle::Result UtilsVk::ensureResourcesInitialized(ContextVk *contextVk, ...@@ -627,7 +626,7 @@ angle::Result UtilsVk::ensureResourcesInitialized(ContextVk *contextVk,
++currentBinding; ++currentBinding;
} }
ANGLE_TRY(renderer->getDescriptorSetLayout( ANGLE_TRY(contextVk->getDescriptorSetLayoutCache().getDescriptorSetLayout(
contextVk, descriptorSetDesc, contextVk, descriptorSetDesc,
&mDescriptorSetLayouts[function][ToUnderlying(DescriptorSetIndex::InternalShader)])); &mDescriptorSetLayouts[function][ToUnderlying(DescriptorSetIndex::InternalShader)]));
...@@ -670,7 +669,7 @@ angle::Result UtilsVk::ensureResourcesInitialized(ContextVk *contextVk, ...@@ -670,7 +669,7 @@ angle::Result UtilsVk::ensureResourcesInitialized(ContextVk *contextVk,
static_cast<uint32_t>(pushConstantsSize)); static_cast<uint32_t>(pushConstantsSize));
} }
ANGLE_TRY(renderer->getPipelineLayout(contextVk, pipelineLayoutDesc, ANGLE_TRY(contextVk->getPipelineLayoutCache().getPipelineLayout(contextVk, pipelineLayoutDesc,
mDescriptorSetLayouts[function], mDescriptorSetLayouts[function],
&mPipelineLayouts[function])); &mPipelineLayouts[function]));
......
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