From 4d5b047c1b31eb57c37078feffa5ab77beec3365 Mon Sep 17 00:00:00 2001 From: moudgils <47460854+moudgils@users.noreply.github.com> Date: Tue, 7 Sep 2021 11:03:11 -0700 Subject: [PATCH] Fix PipelineLibraries (DX12 backend). (#3768) * Fix loading of PipelineLibraries from disk for DX12 backend. Signed-off-by: moudgils * Disabled Saving out PipelineLibraries (for DX12) if pix or Renderdoc is enabled. Addressed some feedback Signed-off-by: moudgils * Fixed an issue withe loading PipelineLibraries and added a cleaner abstraction to not save empty libraries for dx12 Signed-off-by: moudgils --- .../Code/Include/Atom/RHI/PipelineLibrary.h | 78 +++++++++---------- .../RHI/Code/Source/RHI/PipelineLibrary.cpp | 5 ++ .../Code/Source/RHI/PipelineStateCache.cpp | 34 ++++---- .../Platform/Windows/RHI/DX12_Windows.h | 4 +- .../DX12/Code/Source/RHI/PipelineLibrary.cpp | 28 +++++-- .../DX12/Code/Source/RHI/PipelineLibrary.h | 1 + .../Code/Source/RHI/AsyncUploadQueue.cpp | 2 +- .../Include/Atom/RPI.Public/Shader/Shader.h | 8 +- .../Code/Source/RPI.Public/Shader/Shader.cpp | 76 +++++++++--------- 9 files changed, 130 insertions(+), 106 deletions(-) diff --git a/Gems/Atom/RHI/Code/Include/Atom/RHI/PipelineLibrary.h b/Gems/Atom/RHI/Code/Include/Atom/RHI/PipelineLibrary.h index 0c5cfd1182..3a49b121cd 100644 --- a/Gems/Atom/RHI/Code/Include/Atom/RHI/PipelineLibrary.h +++ b/Gems/Atom/RHI/Code/Include/Atom/RHI/PipelineLibrary.h @@ -19,25 +19,24 @@ namespace AZ /// A handle typed to the pipeline library. Used by the PipelineStateCache to abstract access. using PipelineLibraryHandle = Handle; - /** - * PipelineState initialization is an expensive operation on certain platforms. If multiple pipeline states - * are created with little variation between them, the contents are still duplicated. This class is an allocation - * context for pipeline states, provided at PipelineState::Init, which will perform de-duplication of - * internal pipeline state components and cache the results. - * - * Practically speaking, if many pipeline states are created with shared data between them (e.g. permutations - * of the same shader), then providing a PipelineLibrary instance will reduce the memory footprint and cost - * of compilation. - * - * Additionally, the PipelineLibrary is able to serialize the internal driver-contents to and from an opaque - * data blob. This enables building up a pipeline state cache on disk, which can dramatically reduce pipeline - * state compilation cost when run from a pre-warmed cache. - * - * PipelineLibrary is thread-safe, in the sense that it will take a lock during compilation. It is possible - * to initialize pipeline states across threads using the same PipelineLibrary instance, but this will - * result in the two calls serializing on the mutex. Instead, see PipelineStateCache which stores - * a PipelineLibrary instance per thread to avoid this contention. - */ + + //! PipelineState initialization is an expensive operation on certain platforms. If multiple pipeline states + //! are created with little variation between them, the contents are still duplicated. This class is an allocation + //! context for pipeline states, provided at PipelineState::Init, which will perform de-duplication of + //! internal pipeline state components and cache the results. + //! + //! Practically speaking, if many pipeline states are created with shared data between them (e.g. permutations + //! of the same shader), then providing a PipelineLibrary instance will reduce the memory footprint and cost + //! of compilation. + //! + //! Additionally, the PipelineLibrary is able to serialize the internal driver-contents to and from an opaque + //! data blob. This enables building up a pipeline state cache on disk, which can dramatically reduce pipeline + //! state compilation cost when run from a pre-warmed cache. + //! + //! PipelineLibrary is thread-safe, in the sense that it will take a lock during compilation. It is possible + //! to initialize pipeline states across threads using the same PipelineLibrary instance, but this will + //! result in the two calls serializing on the mutex. Instead, see PipelineStateCache which stores + //! a PipelineLibrary instance per thread to avoid this contention. class PipelineLibrary : public DeviceObject { @@ -45,34 +44,31 @@ namespace AZ AZ_RTTI(PipelineLibrary, "{843579BE-57E4-4527-AB00-C0217885AEA9}"); virtual ~PipelineLibrary() = default; - /** - * Initializes the pipeline library from a platform-specific data payload. This data is generated - * by calling GetSerializedData in a previous run of the application. When run for the first - * time, the serialized data should be empty. When the application completes, the library can be - * serialized and the contents saved to disk. Subsequent loads will experience much faster pipeline - * state creation times (on supported platforms). On success, the library is transitioned to the - * initialized state. On failure, the library remains uninitialized. - * @param serializedData The initial serialized data used to initialize the library. It can be null. - */ + //! Initializes the pipeline library from a platform-specific data payload. This data is generated + //! by calling GetSerializedData in a previous run of the application. When run for the first + //! time, the serialized data should be empty. When the application completes, the library can be + //! serialized and the contents saved to disk. Subsequent loads will experience much faster pipeline + //! state creation times (on supported platforms). On success, the library is transitioned to the + //! initialized state. On failure, the library remains uninitialized. + //! @param serializedData The initial serialized data used to initialize the library. It can be null. ResultCode Init(Device& device, const PipelineLibraryData* serializedData); - /** - * Merges the contents of other libraries into this library. This method must be called - * on an initialized library. A common use case for this method is to construct thread-local - * libraries and merge them into a single unified library. The serialized data can then be - * extracted from the unified library. An error code is returned on failure and the behavior - * is as if the method was never called. - */ + //! Merges the contents of other libraries into this library. This method must be called + //! on an initialized library. A common use case for this method is to construct thread-local + //! libraries and merge them into a single unified library. The serialized data can then be + //! extracted from the unified library. An error code is returned on failure and the behavior + //! is as if the method was never called. ResultCode MergeInto(AZStd::array_view librariesToMerge); - /** - * Serializes the platform-specific data and returns it as a new PipelineLibraryData instance. - * The data is opaque to the user and can only be used to re-initialize the library. Use - * this method to extract serialized data prior to application shutdown, save it to disk, and - * use it when initializing on subsequent runs. - */ + //! Serializes the platform-specific data and returns it as a new PipelineLibraryData instance. + //! The data is opaque to the user and can only be used to re-initialize the library. Use + //! this method to extract serialized data prior to application shutdown, save it to disk, and + //! use it when initializing on subsequent runs. ConstPtr GetSerializedData() const; + //! Returns whether the current library need to be merged + virtual bool IsMergeRequired() const; + private: bool ValidateIsInitialized() const; diff --git a/Gems/Atom/RHI/Code/Source/RHI/PipelineLibrary.cpp b/Gems/Atom/RHI/Code/Source/RHI/PipelineLibrary.cpp index 2212ae1591..be0428bc91 100644 --- a/Gems/Atom/RHI/Code/Source/RHI/PipelineLibrary.cpp +++ b/Gems/Atom/RHI/Code/Source/RHI/PipelineLibrary.cpp @@ -72,5 +72,10 @@ namespace AZ return GetSerializedDataInternal(); } + + bool PipelineLibrary::IsMergeRequired() const + { + return true; + } } } diff --git a/Gems/Atom/RHI/Code/Source/RHI/PipelineStateCache.cpp b/Gems/Atom/RHI/Code/Source/RHI/PipelineStateCache.cpp index 52093f7b4c..a2a2736106 100644 --- a/Gems/Atom/RHI/Code/Source/RHI/PipelineStateCache.cpp +++ b/Gems/Atom/RHI/Code/Source/RHI/PipelineStateCache.cpp @@ -166,16 +166,12 @@ namespace AZ } AZStd::unique_lock lock(m_mutex); - const GlobalLibraryEntry& entry = m_globalLibrarySet[handle.GetIndex()]; - /** - * Each thread has its own PipelineLibrary instance. To produce the final serialized data, we - * coalesce data from each individual library by merging the thread-local ones into a single - * global (temporary) library. The data is then extracted from this global library and returned. - * This operation is designed to happen once at application shutdown; certainly not every frame. - */ - + //! Each thread has its own PipelineLibrary instance. To produce the final serialized data, we + //! coalesce data from each individual library by merging the thread-local ones into a single + //! global (temporary) library. The data is then extracted from this global library and returned. + //! This operation is designed to happen once at application shutdown; certainly not every frame. AZStd::vector threadLibraries; m_threadLibrarySet.ForEach([handle, &threadLibraries](const ThreadLibrarySet& threadLibrarySet) { @@ -188,16 +184,26 @@ namespace AZ } }); - Ptr pipelineLibrary = Factory::Get().CreatePipelineLibrary(); - ResultCode resultCode = pipelineLibrary->Init(*m_device, entry.m_serializedData.get()); - - if (resultCode == ResultCode::Success) + bool doesPSODataExist = entry.m_serializedData.get(); + for (const RHI::PipelineLibrary* libraryBase : threadLibraries) { - resultCode = pipelineLibrary->MergeInto(threadLibraries); + const PipelineLibrary* library = static_cast(libraryBase); + doesPSODataExist |= library->IsMergeRequired(); + } + + if (doesPSODataExist) + { + Ptr pipelineLibrary = Factory::Get().CreatePipelineLibrary(); + ResultCode resultCode = pipelineLibrary->Init(*m_device, entry.m_serializedData.get()); if (resultCode == ResultCode::Success) { - return pipelineLibrary->GetSerializedData(); + resultCode = pipelineLibrary->MergeInto(threadLibraries); + + if (resultCode == ResultCode::Success) + { + return pipelineLibrary->GetSerializedData(); + } } } diff --git a/Gems/Atom/RHI/DX12/Code/Source/Platform/Windows/RHI/DX12_Windows.h b/Gems/Atom/RHI/DX12/Code/Source/Platform/Windows/RHI/DX12_Windows.h index b07b9e9e58..3f9079422c 100644 --- a/Gems/Atom/RHI/DX12/Code/Source/Platform/Windows/RHI/DX12_Windows.h +++ b/Gems/Atom/RHI/DX12/Code/Source/Platform/Windows/RHI/DX12_Windows.h @@ -20,8 +20,8 @@ #include -// This define is enabled if winpixeventruntime SDK is downloaded and it's path is hooked up to Environment var ATOM_PIX_PATH. -// Enabling this define will allow the runtime code to add PIX markers which will hel pwith pix and renderdoc gpu captures +// This define is enabled if LY_PIX_ENABLED is enabled during configure. You can use LY_PIX_PATH to point where pix is downloaded. +// Enabling this define will allow the runtime code to add PIX markers which will help with pix and renderdoc gpu captures #ifdef USE_PIX #include #else diff --git a/Gems/Atom/RHI/DX12/Code/Source/RHI/PipelineLibrary.cpp b/Gems/Atom/RHI/DX12/Code/Source/RHI/PipelineLibrary.cpp index 06a0511162..d0a9ef0c07 100644 --- a/Gems/Atom/RHI/DX12/Code/Source/RHI/PipelineLibrary.cpp +++ b/Gems/Atom/RHI/DX12/Code/Source/RHI/PipelineLibrary.cpp @@ -66,15 +66,20 @@ namespace AZ switch (hr) { case D3D12_ERROR_DRIVER_VERSION_MISMATCH: - case DXGI_ERROR_UNSUPPORTED: AZ_Warning("PipelineLibrary", false, "Failed to use pipeline library blob due to driver version mismatch. Contents will be rebuilt."); break; + case DXGI_ERROR_UNSUPPORTED: + AZ_Warning("PipelineLibrary", false, "Failed to use pipeline library blob due to the specified device interface or feature level not supported on this system. Contents will be rebuilt."); + break; case D3D12_ERROR_ADAPTER_NOT_FOUND: AZ_Warning("PipelineLibrary", false, "Failed to use pipeline library blob due to mismatched hardware. Contents will be rebuilt."); break; case E_INVALIDARG: AZ_Assert(false, "Failed to use pipeline library blob due to invalid arguments. Contents will be rebuilt."); break; + case DXGI_ERROR_DEVICE_REMOVED: + AZ_Assert(false, "Failed to use pipeline library blob due to DXGI_ERROR_DEVICE_REMOVED."); + break; default: AZ_Warning("PipelineLibrary", false, "Failed to use pipeline library blob for unknown reason. Contents will be rebuilt."); } @@ -200,23 +205,29 @@ namespace AZ RHI::ResultCode PipelineLibrary::MergeIntoInternal([[maybe_unused]] AZStd::array_view pipelineLibraries) { +#if defined(USE_PIX) || defined(USE_RENDERDOC) + // StorePipeline api does not function properly if Pix or RenderDoc is enabled + return RHI::ResultCode::Fail; +#else + #if defined (AZ_DX12_USE_PIPELINE_LIBRARY) AZStd::lock_guard lock(m_mutex); - for (const RHI::PipelineLibrary* libraryBase : pipelineLibraries) { const PipelineLibrary* library = static_cast(libraryBase); - for (const auto& pipelineStateEntry : library->m_pipelineStates) { - m_library->StorePipeline(pipelineStateEntry.first.c_str(), pipelineStateEntry.second.get()); + if (m_pipelineStates.find(pipelineStateEntry.first) == m_pipelineStates.end()) + { + m_library->StorePipeline(pipelineStateEntry.first.c_str(), pipelineStateEntry.second.get()); - m_pipelineStates.emplace(pipelineStateEntry.first, pipelineStateEntry.second); + m_pipelineStates.emplace(pipelineStateEntry.first, pipelineStateEntry.second); + } } } #endif - return RHI::ResultCode::Success; +#endif } RHI::ConstPtr PipelineLibrary::GetSerializedDataInternal() const @@ -238,5 +249,10 @@ namespace AZ return nullptr; #endif } + + bool PipelineLibrary::IsMergeRequired() const + { + return !m_pipelineStates.empty(); + } } } diff --git a/Gems/Atom/RHI/DX12/Code/Source/RHI/PipelineLibrary.h b/Gems/Atom/RHI/DX12/Code/Source/RHI/PipelineLibrary.h index d43abe4cb1..b970882246 100644 --- a/Gems/Atom/RHI/DX12/Code/Source/RHI/PipelineLibrary.h +++ b/Gems/Atom/RHI/DX12/Code/Source/RHI/PipelineLibrary.h @@ -35,6 +35,7 @@ namespace AZ void ShutdownInternal() override; RHI::ResultCode MergeIntoInternal(AZStd::array_view libraries) override; RHI::ConstPtr GetSerializedDataInternal() const override; + bool IsMergeRequired() const; ////////////////////////////////////////////////////////////////////////// ID3D12DeviceX* m_dx12Device = nullptr; diff --git a/Gems/Atom/RHI/Vulkan/Code/Source/RHI/AsyncUploadQueue.cpp b/Gems/Atom/RHI/Vulkan/Code/Source/RHI/AsyncUploadQueue.cpp index 47cde358ba..7a8499ab9d 100644 --- a/Gems/Atom/RHI/Vulkan/Code/Source/RHI/AsyncUploadQueue.cpp +++ b/Gems/Atom/RHI/Vulkan/Code/Source/RHI/AsyncUploadQueue.cpp @@ -210,7 +210,7 @@ namespace AZ // ImageHeight must be bigger than or equal to the Image's row count. Images with a RowCount that is less than the ImageHeight indicates a block compression. // Images with a RowCount which is higher than the ImageHeight indicates a planar image, which is not supported for streaming images. - AZ_Error("StreamingImage", subresourceLayout.m_size.m_height < subresourceLayout.m_rowCount, "AsyncUploadQueue::QueueUpload expects ImageHeight '%d' to be bigger than or equal to the image's RowCount '%d'.", subresourceLayout.m_size.m_height, subresourceLayout.m_rowCount); + AZ_Error("StreamingImage", subresourceLayout.m_size.m_height >= subresourceLayout.m_rowCount, "AsyncUploadQueue::QueueUpload expects ImageHeight '%d' to be bigger than or equal to the image's RowCount '%d'.", subresourceLayout.m_size.m_height, subresourceLayout.m_rowCount); // The final staging size for each CopyTextureRegion command uint32_t stagingSize = stagingSlicePitch; diff --git a/Gems/Atom/RPI/Code/Include/Atom/RPI.Public/Shader/Shader.h b/Gems/Atom/RPI/Code/Include/Atom/RPI.Public/Shader/Shader.h index 6684eec0f7..e2568f3b61 100644 --- a/Gems/Atom/RPI/Code/Include/Atom/RPI.Public/Shader/Shader.h +++ b/Gems/Atom/RPI/Code/Include/Atom/RPI.Public/Shader/Shader.h @@ -18,7 +18,7 @@ #include #include - +#include #include namespace AZ @@ -175,9 +175,6 @@ namespace AZ // And of course we don't need to handle OnShaderReinitialized because this *is* this Shader. /////////////////////////////////////////////////////////////////// - //! Returns the path to the pipeline library cache file. - AZStd::string GetPipelineLibraryPath() const; - //! A strong reference to the shader asset. Data::Asset m_asset; @@ -206,6 +203,9 @@ namespace AZ //! DrawListTag associated with this shader. RHI::DrawListTag m_drawListTag; + + //! PipelineLibrary file name + char m_pipelineLibraryPath[AZ_MAX_PATH_LEN] = { 0 }; }; } } diff --git a/Gems/Atom/RPI/Code/Source/RPI.Public/Shader/Shader.cpp b/Gems/Atom/RPI/Code/Source/RPI.Public/Shader/Shader.cpp index 4440b96f79..e602e3e91b 100644 --- a/Gems/Atom/RPI/Code/Source/RPI.Public/Shader/Shader.cpp +++ b/Gems/Atom/RPI/Code/Source/RPI.Public/Shader/Shader.cpp @@ -7,18 +7,14 @@ */ #include -#include - -#include - -#include #include - +#include +#include #include - -#include -#include #include +#include +#include + namespace AZ { @@ -75,6 +71,29 @@ namespace AZ Shutdown(); } + static bool GetPipelineLibraryPath(char* pipelineLibraryPath, size_t pipelineLibraryPathLength, const ShaderAsset& shaderAsset) + { + if (auto* fileIOBase = IO::FileIOBase::GetInstance()) + { + const Data::AssetId& assetId = shaderAsset.GetId(); + + Name platformName = RHI::Factory::Get().GetName(); + Name shaderName = shaderAsset.GetName(); + + AZStd::string uuidString; + assetId.m_guid.ToString(uuidString, false, false); + + char pipelineLibraryPathTemp[AZ_MAX_PATH_LEN]; + azsnprintf( + pipelineLibraryPathTemp, AZ_MAX_PATH_LEN, "@user@/Atom/PipelineStateCache/%s/%s_%s_%d.bin", platformName.GetCStr(), + shaderName.GetCStr(), uuidString.data(), assetId.m_subId); + + fileIOBase->ResolvePath(pipelineLibraryPathTemp, pipelineLibraryPath, pipelineLibraryPathLength); + return true; + } + return false; + } + RHI::ResultCode Shader::Init(ShaderAsset& shaderAsset) { Data::AssetBus::Handler::BusDisconnect(); @@ -87,6 +106,8 @@ namespace AZ m_asset = { &shaderAsset, AZ::Data::AssetLoadBehavior::PreLoad }; m_pipelineStateType = shaderAsset.GetPipelineStateType(); + GetPipelineLibraryPath(m_pipelineLibraryPath, AZ_MAX_PATH_LEN, *m_asset); + { AZStd::unique_lock lock(m_variantCacheMutex); m_shaderVariants.clear(); @@ -123,7 +144,7 @@ namespace AZ AZ_Error("Shader", false, "Failed to acquire a DrawListTag. Entries are full."); } } - + ShaderVariantFinderNotificationBus::Handler::BusConnect(m_asset.GetId()); Data::AssetBus::Handler::BusConnect(m_asset.GetId()); ShaderReloadNotificationBus::Handler::BusConnect(m_asset.GetId()); @@ -249,49 +270,28 @@ namespace AZ } } /////////////////////////////////////////////////////////////////// - - + ConstPtr Shader::LoadPipelineLibrary() const - { - if (IO::FileIOBase::GetInstance()) + { + if (m_pipelineLibraryPath[0] != 0) { - return Utils::LoadObjectFromFile(GetPipelineLibraryPath()); + return Utils::LoadObjectFromFile(m_pipelineLibraryPath); } return nullptr; } void Shader::SavePipelineLibrary() const { - if (auto* fileIOBase = IO::FileIOBase::GetInstance()) + if (m_pipelineLibraryPath[0] != 0) { RHI::ConstPtr serializedData = m_pipelineStateCache->GetLibrarySerializedData(m_pipelineLibraryHandle); if (serializedData) { - const AZStd::string pipelineLibraryPath = GetPipelineLibraryPath(); - - char pipelineLibraryPathResolved[AZ_MAX_PATH_LEN] = { 0 }; - fileIOBase->ResolvePath(pipelineLibraryPath.c_str(), pipelineLibraryPathResolved, AZ_MAX_PATH_LEN); - Utils::SaveObjectToFile(pipelineLibraryPathResolved, DataStream::ST_BINARY, serializedData.get()); + Utils::SaveObjectToFile(m_pipelineLibraryPath, DataStream::ST_BINARY, serializedData.get()); } } - else - { - AZ_Error("Shader", false, "FileIOBase is not initialized"); - } } - - AZStd::string Shader::GetPipelineLibraryPath() const - { - const Data::InstanceId& instanceId = GetId(); - Name platformName = RHI::Factory::Get().GetName(); - Name shaderName = m_asset->GetName(); - - AZStd::string uuidString; - instanceId.m_guid.ToString(uuidString, false, false); - - return AZStd::string::format("@user@/Atom/PipelineStateCache/%s/%s_%s_%d.bin", platformName.GetCStr(), shaderName.GetCStr(), uuidString.data(), instanceId.m_subId); - } - + ShaderOptionGroup Shader::CreateShaderOptionGroup() const { return ShaderOptionGroup(m_asset->GetShaderOptionGroupLayout());