From e193e5b3538bc4c7eaade2ab1232f6502f7c9dc3 Mon Sep 17 00:00:00 2001 From: Artur K <96597+nemerle@users.noreply.github.com> Date: Fri, 6 Aug 2021 03:15:37 +0200 Subject: [PATCH] EnvironmentVariableHolder: reduce the size of template instantiation. (#2857) * EnvironmentVariableHolder: reduce the size of template instantiation. Move almost all destruction logic to EnvironmentVariableHolderBase::UnregisterAndDestroy. Specialized templates have DestructDispatchNoLock instead that can either destroy the held value, or the holder itself. UnregisterAndDestroy has been moved to the cpp file. All of these changes reduce the profile build time and size on linux Here, the size of bin/profile goes down by ~200MB. Signed-off-by: nemerle <96597+nemerle@users.noreply.github.com> * Requested changes/fixups. Signed-off-by: nemerle <96597+nemerle@users.noreply.github.com> * Use scoped_lock to simplify mutex management. Updated comments. Signed-off-by: nemerle <96597+nemerle@users.noreply.github.com> * Hopefully a fix for env variables released at a wrong time Conditional was using incorrect variable Signed-off-by: nemerle <96597+nemerle@users.noreply.github.com> * Comment fixup Signed-off-by: nemerle <96597+nemerle@users.noreply.github.com> * Missing negation in conditional Signed-off-by: nemerle <96597+nemerle@users.noreply.github.com> * Cleanup the internal logic in UnregisterAndDestroy Signed-off-by: nemerle <96597+nemerle@users.noreply.github.com> --- .../AzCore/AzCore/Module/Environment.cpp | 39 +++++++- .../AzCore/AzCore/Module/Environment.h | 92 ++++++------------- 2 files changed, 67 insertions(+), 64 deletions(-) diff --git a/Code/Framework/AzCore/AzCore/Module/Environment.cpp b/Code/Framework/AzCore/AzCore/Module/Environment.cpp index f3a3533f4a..bec530e2d1 100644 --- a/Code/Framework/AzCore/AzCore/Module/Environment.cpp +++ b/Code/Framework/AzCore/AzCore/Module/Environment.cpp @@ -11,6 +11,7 @@ #include #include #include +#include namespace AZ { @@ -75,6 +76,42 @@ namespace AZ bool operator==(const OSStdAllocator& a, const OSStdAllocator& b) { (void)a; (void)b; return true; } bool operator!=(const OSStdAllocator& a, const OSStdAllocator& b) { (void)a; (void)b; return false; } + void EnvironmentVariableHolderBase::UnregisterAndDestroy(DestructFunc destruct, bool moduleRelease) + { + const bool releaseByUseCount = (--m_useCount == 0); + // We take over the lock, and release it before potentially destroying/freeing ourselves + { + AZStd::scoped_lock envLockHolder(AZStd::adopt_lock, m_mutex); + const bool releaseByModule = (moduleRelease && !m_canTransferOwnership && m_moduleOwner == AZ::Environment::GetModuleId()); + + if (!releaseByModule && !releaseByUseCount) + { + return; + } + // if the environment that created us is gone the owner can be null + // which means (assuming intermodule allocator) that the variable is still alive + // but can't be found as it's not part of any environment. + if (m_environmentOwner) + { + m_environmentOwner->RemoveVariable(m_guid); + m_environmentOwner = nullptr; + } + if (m_isConstructed) + { + destruct(this, DestroyTarget::Member); // destruct the value + } + } + // m_mutex is no longer held here, envLockHolder has released it above. + if (releaseByUseCount) + { + // m_mutex is unlocked before this is deleted + Environment::AllocatorInterface* allocator = m_allocator; + // Call child class dtor and clear the memory + destruct(this, DestroyTarget::Self); + allocator->DeAllocate(this); + } + } + // instance of the environment EnvironmentInterface* EnvironmentInterface::s_environment = nullptr; @@ -110,7 +147,7 @@ namespace AZ #ifdef AZ_ENVIRONMENT_VALIDATE_ON_EXIT AZ_Assert(m_numAttached == 0, "We should not delete an environment while there are %d modules attached! Unload all DLLs first!", m_numAttached); #endif - + for (auto variableIt : m_variableMap) { EnvironmentVariableHolderBase* holder = reinterpret_cast(variableIt.second); diff --git a/Code/Framework/AzCore/AzCore/Module/Environment.h b/Code/Framework/AzCore/AzCore/Module/Environment.h index b0711b1bbe..e87ff81446 100644 --- a/Code/Framework/AzCore/AzCore/Module/Environment.h +++ b/Code/Framework/AzCore/AzCore/Module/Environment.h @@ -200,6 +200,11 @@ namespace AZ class EnvironmentVariableHolderBase { friend class EnvironmentImpl; + protected: + enum class DestroyTarget { + Member, + Self + }; public: EnvironmentVariableHolderBase(u32 guid, AZ::Internal::EnvironmentInterface* environmentOwner, bool canOwnershipTransfer, Environment::AllocatorInterface* allocator) : m_environmentOwner(environmentOwner) @@ -217,12 +222,21 @@ namespace AZ return m_isConstructed; } + bool IsOwner() const + { + return m_moduleOwner == Environment::GetModuleId(); + } + u32 GetId() const { return m_guid; } - protected: + using DestructFunc = void (*)(EnvironmentVariableHolderBase *, DestroyTarget); + // Assumes the m_mutex is already locked. + // On return m_mutex is in an unlocked state. + void UnregisterAndDestroy(DestructFunc destruct, bool moduleRelease); + AZ::Internal::EnvironmentInterface* m_environmentOwner; ///< Used to know which environment we should use to free the variable if we can't transfer ownership void* m_moduleOwner; ///< Used when the variable can't transfered across module and we need to destruct the variable when the module is going away bool m_canTransferOwnership; ///< True if variable can be allocated in one module and freed in other. Usually true for POD types when they share allocator. @@ -242,41 +256,29 @@ namespace AZ memset(&m_value, 0, sizeof(T)); } - template + template void ConstructImpl(const AZStd::false_type& /* AZStd::has_trivial_constructor */, Args&&... args) { // Construction of non-trivial types is left up to the type's constructor. new(&m_value) T(AZStd::forward(args)...); } - - void DestructImpl(const AZStd::true_type& /* AZStd::is_trivially_destructible */) + static void DestructDispatchNoLock(EnvironmentVariableHolderBase *base, DestroyTarget selfDestruct) { - // do nothing - } - - void DestructImpl(const AZStd::false_type& /* AZStd::is_trivially_destructible */) - { - reinterpret_cast(&m_value)->~T(); - } - - // Assumes the lock is already held - void UnregisterAndDestruct() - { - // if the environment that created us is gone the owner can be null - // which means (assuming intermodule allocator) that the variable is still alive - // but can't be found as it's not part of any environment. - if (m_environmentOwner) + auto *self = reinterpret_cast(base); + if (selfDestruct == DestroyTarget::Self) { - m_environmentOwner->RemoveVariable(m_guid); - m_environmentOwner = nullptr; + self->~EnvironmentVariableHolder(); + return; } - if (m_isConstructed) + AZ_Assert(self->m_isConstructed, "Variable is not constructed. Please check your logic and guard if needed!"); + self->m_isConstructed = false; + self->m_moduleOwner = nullptr; + if constexpr(!AZStd::is_trivially_destructible_v) { - DestructNoLock(); + reinterpret_cast(&self->m_value)->~T(); } } - public: EnvironmentVariableHolder(u32 guid, bool isOwnershipTransfer, Environment::AllocatorInterface* allocator) : EnvironmentVariableHolderBase(guid, Environment::GetInstance(), isOwnershipTransfer, allocator) @@ -287,12 +289,6 @@ namespace AZ { AZ_Assert(!m_isConstructed, "To get the destructor we should have already destructed the variable!"); } - - bool IsOwner() const - { - return m_moduleOwner == Environment::GetModuleId(); - } - void AddRef() { AZStd::lock_guard lock(m_mutex); @@ -303,30 +299,8 @@ namespace AZ void Release() { m_mutex.lock(); - - if (--s_moduleUseCount == 0) - { - if (!m_canTransferOwnership && m_moduleOwner == AZ::Environment::GetModuleId()) - { - UnregisterAndDestruct(); - } - } - - if (--m_useCount == 0) - { - UnregisterAndDestruct(); - - // unlock before this is deleted - m_mutex.unlock(); - - Environment::AllocatorInterface* allocator = m_allocator; - // Call dtor and clear the memory - this->~EnvironmentVariableHolder(); - allocator->DeAllocate(this); - return; - } - - m_mutex.unlock(); + const bool moduleRelease = (--s_moduleUseCount == 0); + UnregisterAndDestroy(DestructDispatchNoLock, moduleRelease); } void Construct() @@ -352,18 +326,10 @@ namespace AZ } } - void DestructNoLock() - { - AZ_Assert(m_isConstructed, "Variable is not constructed. Please check your logic and guard if needed!"); - m_isConstructed = false; - m_moduleOwner = nullptr; - DestructImpl(typename AZStd::is_trivially_destructible::type()); - } - void Destruct() { AZStd::lock_guard lock(m_mutex); - DestructNoLock(); + DestructDispatchNoLock(this, DestroyTarget::Member); } // variable storage