DeviceObjectArchive: reworked Deserialize method to return error instead of throwing exception

Author: TheMostDiligent

Graphics/GraphicsEngine/include/DeviceObjectArchive.hpp

@@ -242,7 +242,7 @@ class DeviceObjectArchive
     void AppendDeviceData(const DeviceObjectArchive& Src, DeviceType Dev) noexcept(false);
     void Merge(const DeviceObjectArchive& Src) noexcept(false);
 
-    void Deserialize(const CreateInfo& CI) noexcept(false);
+    bool Deserialize(const CreateInfo& CI) noexcept;
     void Serialize(IFileStream* pStream) const;
     void Serialize(IDataBlob** ppDataBlob) const;
 
@@ -300,6 +300,8 @@ class DeviceObjectArchive
         return m_NamedResources;
     }
 
+    void Clear() noexcept;
+
 private:
     // Named resources
     std::unordered_map<NamedResourceKey, ResourceData, NamedResourceKey::Hasher> m_NamedResources;

Graphics/GraphicsEngine/src/DearchiverBase.cpp

@@ -653,52 +653,47 @@ bool DearchiverBase::LoadArchive(const IDataBlob* pArchiveData, Uint32 ContentVe
     if (pArchiveData == nullptr)
         return false;
 
-    try
+    for (const ArchiveData& Archive : m_Archives)
     {
-        for (const auto& Archive : m_Archives)
+        if (Archive.pObjArchive->GetData() == pArchiveData)
         {
-            if (Archive.pObjArchive->GetData() == pArchiveData)
-            {
-                // The archive is already loaded
-                return true;
-            }
+            // The archive is already loaded
+            return true;
         }
+    }
 
-        auto       pObjArchive = std::make_unique<DeviceObjectArchive>(DeviceObjectArchive::CreateInfo{pArchiveData, ContentVersion, MakeCopy});
-        const auto ArchiveIdx  = m_Archives.size();
+    std::unique_ptr<DeviceObjectArchive> pObjArchive = std::make_unique<DeviceObjectArchive>();
+    if (!pObjArchive->Deserialize(DeviceObjectArchive::CreateInfo{pArchiveData, ContentVersion, MakeCopy}))
+        return false;
+
+    const size_t ArchiveIdx = m_Archives.size();
 
-        const auto& ArchiveResources = pObjArchive->GetNamedResources();
-        for (const auto& it : ArchiveResources)
+    const auto& ArchiveResources = pObjArchive->GetNamedResources();
+    for (const auto& it : ArchiveResources)
+    {
+        const ResourceType ResType      = it.first.GetType();
+        const char*        ResName      = it.first.GetName();
+        constexpr bool     MakeNameCopy = true;
+
+        const auto it_inserted = m_ResNameToArchiveIdx.emplace(NamedResourceKey{ResType, ResName, MakeNameCopy}, ArchiveIdx);
+        if (!it_inserted.second)
         {
-            const auto     ResType      = it.first.GetType();
-            const auto*    ResName      = it.first.GetName();
-            constexpr auto MakeNameCopy = true;
+            const auto& OtherArchiveResources = m_Archives[it_inserted.first->second].pObjArchive->GetNamedResources();
+            const auto  it_other              = OtherArchiveResources.find(NamedResourceKey{ResType, ResName});
 
-            const auto it_inserted = m_ResNameToArchiveIdx.emplace(NamedResourceKey{ResType, ResName, MakeNameCopy}, ArchiveIdx);
-            if (!it_inserted.second)
+            const bool IsDuplicate =
+                (it_other != OtherArchiveResources.end()) &&
+                (it.second == it_other->second);
+            if (!IsDuplicate)
             {
-                const auto& OtherArchiveResources = m_Archives[it_inserted.first->second].pObjArchive->GetNamedResources();
-                const auto  it_other              = OtherArchiveResources.find(NamedResourceKey{ResType, ResName});
-
-                const auto IsDuplicate =
-                    (it_other != OtherArchiveResources.end()) &&
-                    (it.second == it_other->second);
-                if (!IsDuplicate)
-                {
-                    LOG_ERROR_MESSAGE("Resource with name '", ResName, "' already exists in the archive.");
-                }
+                LOG_ERROR_MESSAGE("Resource with name '", ResName, "' already exists in the archive.");
             }
         }
+    }
 
-        m_Archives.emplace_back(std::move(pObjArchive));
+    m_Archives.emplace_back(std::move(pObjArchive));
 
-        return true;
-    }
-    catch (...)
-    {
-        LOG_ERROR("Failed to create the device object archive");
-        return false;
-    }
+    return true;
 }
 
 void DearchiverBase::UnpackPipelineState(const PipelineStateUnpackInfo& UnpackInfo, IPipelineState** ppPSO)

Graphics/GraphicsEngine/src/DeviceObjectArchive.cpp

@@ -151,8 +151,36 @@ DeviceObjectArchive::DeviceObjectArchive(Uint32 ContentVersion) noexcept :
 {
 }
 
-void DeviceObjectArchive::Deserialize(const CreateInfo& CI) noexcept(false)
+void DeviceObjectArchive::Clear() noexcept
 {
+    m_NamedResources.clear();
+    m_DeviceShaders = {};
+    m_pArchiveData.Release();
+    m_ContentVersion = 0;
+}
+
+
+bool DeviceObjectArchive::Deserialize(const CreateInfo& CI) noexcept
+{
+    Clear();
+
+#define CHECK_ARCHIVE(Expr, ...)            \
+    do                                      \
+    {                                       \
+        if (!(Expr))                        \
+        {                                   \
+            LOG_ERROR_MESSAGE(__VA_ARGS__); \
+            Clear();                        \
+            return false;                   \
+        }                                   \
+    } while (false)
+
+    CHECK_ARCHIVE(CI.pData != nullptr, "pData must not be null");
+
+    m_pArchiveData = CI.MakeCopy ?
+        DataBlobImpl::MakeCopy(CI.pData) :
+        const_cast<IDataBlob*>(CI.pData); // Need to remove const for AddRef/Release
+
     Serializer<SerializerMode::Read> Reader{
         SerializedData{
             const_cast<void*>(CI.pData->GetConstDataPtr()),
@@ -164,56 +192,48 @@ void DeviceObjectArchive::Deserialize(const CreateInfo& CI) noexcept(false)
     // NB: this must match header serialization in DeviceObjectArchive::SerializeHeader
     ArchiveHeader Header;
     ASSERT_SIZEOF64(Header, 24, "Please handle new members here");
-    if (!ArchiveReader.Ser(Header.MagicNumber))
-        LOG_ERROR_AND_THROW("Failed to read device object archive header magic number.");
+    CHECK_ARCHIVE(ArchiveReader.Ser(Header.MagicNumber), "Failed to read device object archive header magic number.");
 
-    if (Header.MagicNumber != HeaderMagicNumber)
-        LOG_ERROR_AND_THROW("Invalid device object archive header.");
+    CHECK_ARCHIVE(Header.MagicNumber == HeaderMagicNumber, "Invalid device object archive header.");
 
-    if (!ArchiveReader.Ser(Header.Version))
-        LOG_ERROR_AND_THROW("Failed to read device object archive version.");
+    CHECK_ARCHIVE(ArchiveReader.Ser(Header.Version), "Failed to read device object archive version.");
 
-    if (Header.Version != ArchiveVersion)
-        LOG_ERROR_AND_THROW("Unsupported device object archive version: ", Header.Version, ". Expected version: ", Uint32{ArchiveVersion});
+    CHECK_ARCHIVE(Header.Version == ArchiveVersion, "Unsupported device object archive version: ", Header.Version, ". Expected version: ", Uint32{ArchiveVersion});
 
-    if (!ArchiveReader.Ser(Header.APIVersion))
-        LOG_ERROR_AND_THROW("Failed to read Diligent API version.");
+    CHECK_ARCHIVE(ArchiveReader.Ser(Header.APIVersion), "Failed to read Diligent API version.");
 
-    if (!ArchiveReader.Ser(Header.ContentVersion))
-        LOG_ERROR_AND_THROW("Failed to read device object archive content version.");
+    CHECK_ARCHIVE(ArchiveReader.Ser(Header.ContentVersion), "Failed to read device object archive content version.");
 
-    if (CI.ContentVersion != CreateInfo{}.ContentVersion && Header.ContentVersion != CI.ContentVersion)
-        LOG_ERROR_AND_THROW("Invalid archive content version: ", Header.ContentVersion, ". Expected version: ", CI.ContentVersion);
+    CHECK_ARCHIVE(CI.ContentVersion == CreateInfo{}.ContentVersion || Header.ContentVersion == CI.ContentVersion,
+                  "Invalid archive content version: ", Header.ContentVersion, ". Expected version: ", CI.ContentVersion);
     m_ContentVersion = Header.ContentVersion;
 
-    if (!ArchiveReader.Ser(Header.GitHash))
-        LOG_ERROR_AND_THROW("Failed to read Git Hash.");
+    CHECK_ARCHIVE(ArchiveReader.Ser(Header.GitHash), "Failed to read Git Hash.");
 
     Uint32 NumResources = 0;
-    if (!Reader(NumResources))
-        LOG_ERROR_AND_THROW("Failed to read the number of named resources in the device object archive.");
+    CHECK_ARCHIVE(Reader(NumResources), "Failed to read the number of named resources in the device object archive.");
 
     for (Uint32 res = 0; res < NumResources; ++res)
     {
         const char*  Name    = nullptr;
         ResourceType ResType = ResourceType::Undefined;
-        if (!Reader(ResType, Name))
-            LOG_ERROR_AND_THROW("Failed to read the type and name of resource ", res, "/", NumResources, '.');
+        CHECK_ARCHIVE(Reader(ResType, Name), "Failed to read the type and name of resource ", res, "/", NumResources, '.');
         VERIFY_EXPR(Name != nullptr);
 
         // No need to make the name copy as we keep the source data blob alive.
-        constexpr auto MakeNameCopy = false;
-        auto&          ResData      = m_NamedResources[NamedResourceKey{ResType, Name, MakeNameCopy}];
+        constexpr bool MakeNameCopy = false;
+        ResourceData&  ResData      = m_NamedResources[NamedResourceKey{ResType, Name, MakeNameCopy}];
 
-        if (!ArchiveReader.SerializeResourceData(ResData))
-            LOG_ERROR_AND_THROW("Failed to read data of resource '", Name, "'.");
+        CHECK_ARCHIVE(ArchiveReader.SerializeResourceData(ResData), "Failed to read data of resource '", Name, "'.");
     }
 
-    for (auto& Shaders : m_DeviceShaders)
+    for (std::vector<SerializedData>& Shaders : m_DeviceShaders)
     {
-        if (!ArchiveReader.SerializeShaders(Shaders))
-            LOG_ERROR_AND_THROW("Failed to read shader data from the device object archive.");
+        CHECK_ARCHIVE(ArchiveReader.SerializeShaders(Shaders), "Failed to read shader data from the device object archive.");
     }
+#undef CHECK_ARCHIVE
+
+    return true;
 }
 
 void DeviceObjectArchive::Serialize(IDataBlob** ppDataBlob) const
@@ -320,17 +340,12 @@ const char* ResourceTypeToString(DeviceObjectArchive::ResourceType Type)
 } // namespace
 
 
-DeviceObjectArchive::DeviceObjectArchive(const CreateInfo& CI) noexcept(false) :
-    m_pArchiveData{
-        CI.MakeCopy ?
-            DataBlobImpl::MakeCopy(CI.pData) :
-            const_cast<IDataBlob*>(CI.pData) // Need to remove const for AddRef/Release
-    }
+DeviceObjectArchive::DeviceObjectArchive(const CreateInfo& CI) noexcept(false)
 {
-    if (!m_pArchiveData)
-        LOG_ERROR_AND_THROW("pData must not be null");
-
-    Deserialize(CI);
+    if (!Deserialize(CI))
+    {
+        LOG_ERROR_AND_THROW("Failed to deserialize device object archive");
+    }
 }
 
 const SerializedData& DeviceObjectArchive::GetDeviceSpecificData(ResourceType Type,