From 18b947fd0016146f1399d7e5bba3992a023cf979 Mon Sep 17 00:00:00 2001 From: lumberyard-employee-dm <56135373+lumberyard-employee-dm@users.noreply.github.com> Date: Fri, 1 Oct 2021 12:11:55 -0500 Subject: [PATCH] Changed the AssetManager DispatchEvents function to continously pump the (#4432) AssetBus of queued functions until empty. This replicates the old behavior of the EBusQueuePolicy::Execute function that would continue to execute functions if new ones were added during the execution of the current queue. Split the TestFixture class from the AssetHandler and EBus handler for the DynamicSliceInstanceSpawnerTests and PrefabInstanceSpawnerTest. This avoids the AssetMananager destructor from deleting the test fixture if the call to UnregisterHandler is ever removed. This also allows the memory allocators to get online earlier. Signed-off-by: lumberyard-employee-dm <56135373+lumberyard-employee-dm@users.noreply.github.com> --- .../AzCore/AzCore/Asset/AssetManager.cpp | 7 +- .../DynamicSliceInstanceSpawnerTests.cpp | 143 ++++++++++-------- .../Code/Tests/PrefabInstanceSpawnerTests.cpp | 143 ++++++++++-------- Gems/Vegetation/Code/Tests/VegetationTest.h | 25 ++- 4 files changed, 180 insertions(+), 138 deletions(-) diff --git a/Code/Framework/AzCore/AzCore/Asset/AssetManager.cpp b/Code/Framework/AzCore/AzCore/Asset/AssetManager.cpp index 8eb620f69e..06bb0b0cac 100644 --- a/Code/Framework/AzCore/AzCore/Asset/AssetManager.cpp +++ b/Code/Framework/AzCore/AzCore/Asset/AssetManager.cpp @@ -551,8 +551,6 @@ namespace AZ { PrepareShutDown(); - DispatchEvents(); - // Acquire the asset lock to make sure nobody else is trying to do anything fancy with assets AZStd::scoped_lock assetLock(m_assetMutex); @@ -575,7 +573,10 @@ namespace AZ { AZ_PROFILE_FUNCTION(AzCore); AssetManagerNotificationBus::Broadcast(&AssetManagerNotificationBus::Events::OnAssetEventsDispatchBegin); - AssetBus::ExecuteQueuedEvents(); + while (AssetBus::QueuedEventCount()) + { + AssetBus::ExecuteQueuedEvents(); + } AssetManagerNotificationBus::Broadcast(&AssetManagerNotificationBus::Events::OnAssetEventsDispatchEnd); } diff --git a/Gems/Vegetation/Code/Tests/DynamicSliceInstanceSpawnerTests.cpp b/Gems/Vegetation/Code/Tests/DynamicSliceInstanceSpawnerTests.cpp index 9838a7b6a1..26d8028522 100644 --- a/Gems/Vegetation/Code/Tests/DynamicSliceInstanceSpawnerTests.cpp +++ b/Gems/Vegetation/Code/Tests/DynamicSliceInstanceSpawnerTests.cpp @@ -50,80 +50,25 @@ namespace UnitTest } }; - // To test Dynamic Slice spawning, we need to mock up enough of the asset management system and the dynamic slice - // asset handling to pretend like we're loading/unloading dynamic slices successfully. - class DynamicSliceInstanceSpawnerTests - : public VegetationComponentTests - , public UnitTest::SetRestoreFileIOBaseRAII - , public Vegetation::DescriptorNotificationBus::Handler + class DynamicSliceAssetCatalogAndHandler + : public Vegetation::DescriptorNotificationBus::Handler , public AZ::Data::AssetCatalogRequestBus::Handler , public AZ::Data::AssetHandler , public AZ::Data::AssetCatalog , public AzFramework::SliceGameEntityOwnershipServiceRequestBus::Handler { public: - DynamicSliceInstanceSpawnerTests() - : UnitTest::SetRestoreFileIOBaseRAII(m_fileIOMock) + DynamicSliceAssetCatalogAndHandler() { - AZ::IO::MockFileIOBase::InstallDefaultReturns(m_fileIOMock); - } - - void RegisterComponentDescriptors() override - { - m_app.RegisterComponentDescriptor(MockDynamicSliceInstanceVegetationSystemComponent::CreateDescriptor()); - } - - void SetUp() override - { - VegetationComponentTests::SetUp(); - - // Create a real Asset Mananger, and point to ourselves as the handler for DynamicSliceAsset. - AZ::AllocatorInstance::Create(); - AZ::AllocatorInstance::Create(); - - - // Initialize the job manager with 1 thread for the AssetManager to use. - AZ::JobManagerDesc jobDesc; - AZ::JobManagerThreadDesc threadDesc; - jobDesc.m_workerThreads.push_back(threadDesc); - m_jobManager = aznew AZ::JobManager(jobDesc); - m_jobContext = aznew AZ::JobContext(*m_jobManager); - AZ::JobContext::SetGlobalContext(m_jobContext); - - AZ::Data::AssetManager::Descriptor descriptor; - AZ::Data::AssetManager::Create(descriptor); - AZ::Data::AssetManager::Instance().RegisterHandler(this, AZ::AzTypeInfo::Uuid()); - AZ::Data::AssetManager::Instance().RegisterCatalog(this, AZ::AzTypeInfo::Uuid()); - - m_app.RegisterComponentDescriptor(AZ::SliceComponent::CreateDescriptor()); - // Intercept messages for finding assets by name and creating/destroying slices. AZ::Data::AssetCatalogRequestBus::Handler::BusConnect(); AzFramework::SliceGameEntityOwnershipServiceRequestBus::Handler::BusConnect(); } - void TearDown() override + ~DynamicSliceAssetCatalogAndHandler() { - // Give the AssetManager a chance to fire off any lingering events and perform cleanup for any - // dynamic slice assets we loaded. - AZ::Data::AssetManager::Instance().DispatchEvents(); - AzFramework::SliceGameEntityOwnershipServiceRequestBus::Handler::BusDisconnect(); - AZ::Data::AssetManager::Instance().UnregisterCatalog(this); - AZ::Data::AssetManager::Instance().UnregisterHandler(this); - AZ::Data::AssetCatalogRequestBus::Handler::BusDisconnect(); - - AZ::Data::AssetManager::Destroy(); - - AZ::JobContext::SetGlobalContext(nullptr); - delete m_jobContext; - delete m_jobManager; - - AZ::AllocatorInstance::Destroy(); - AZ::AllocatorInstance::Destroy(); - - VegetationComponentTests::TearDown(); } // Helper methods: @@ -207,7 +152,7 @@ namespace UnitTest AZStd::string GetAssetPathById(const AZ::Data::AssetId& /*id*/) override { return m_assetPath; } AZ::Data::AssetId GetAssetIdByPath(const char* /*path*/, const AZ::Data::AssetType& /*typeToRegister*/, bool /*autoRegisterIfNotFound*/) override { return m_assetId; } AZ::Data::AssetInfo GetAssetInfoById(const AZ::Data::AssetId& /*id*/) override - { + { AZ::Data::AssetInfo assetInfo; assetInfo.m_assetId = m_assetId; assetInfo.m_assetType = AZ::AzTypeInfo::Uuid(); @@ -244,9 +189,77 @@ namespace UnitTest AZStd::string m_assetPath; AZ::Data::AssetId m_assetId; int m_numOnLoadedCalls = 0; + }; + // To test Dynamic Slice spawning, we need to mock up enough of the asset management system and the dynamic slice + // asset handling to pretend like we're loading/unloading dynamic slices successfully. + class DynamicSliceInstanceSpawnerTests + : public VegetationComponentTests + { + public: + DynamicSliceInstanceSpawnerTests() + : m_restoreFileIO(m_fileIOMock) + { + AZ::IO::MockFileIOBase::InstallDefaultReturns(m_fileIOMock); + } + void SetUp() override + { + VegetationComponentTests::SetUp(); + + // Create a real Asset Mananger, and point to ourselves as the handler for DynamicSliceAsset. + AZ::AllocatorInstance::Create(); + AZ::AllocatorInstance::Create(); + + // Initialize the job manager with 1 thread for the AssetManager to use. + AZ::JobManagerDesc jobDesc; + AZ::JobManagerThreadDesc threadDesc; + jobDesc.m_workerThreads.push_back(threadDesc); + m_jobManager = aznew AZ::JobManager(jobDesc); + m_jobContext = aznew AZ::JobContext(*m_jobManager); + AZ::JobContext::SetGlobalContext(m_jobContext); + + AZ::Data::AssetManager::Descriptor descriptor; + AZ::Data::AssetManager::Create(descriptor); + m_testHandler = AZStd::make_unique(); + AZ::Data::AssetManager::Instance().RegisterHandler(m_testHandler.get(), AZ::AzTypeInfo::Uuid()); + AZ::Data::AssetManager::Instance().RegisterCatalog(m_testHandler.get(), AZ::AzTypeInfo::Uuid()); + + m_app.RegisterComponentDescriptor(AZ::SliceComponent::CreateDescriptor()); + } + + void TearDown() override + { + // Clear out the list of queued AssetBus Events before unregistering the AssetHandler + // to make sure pending references to Asset instances are cleared + AZ::Data::AssetManager::Instance().DispatchEvents(); + AZ::Data::AssetManager::Instance().UnregisterHandler(m_testHandler.get()); + AZ::Data::AssetManager::Instance().UnregisterCatalog(m_testHandler.get()); + AZ::Data::AssetManager::Destroy(); + + m_testHandler.reset(); + + AZ::JobContext::SetGlobalContext(nullptr); + delete m_jobContext; + delete m_jobManager; + + AZ::AllocatorInstance::Destroy(); + AZ::AllocatorInstance::Destroy(); + + VegetationComponentTests::TearDown(); + } + + void RegisterComponentDescriptors() override + { + m_app.RegisterComponentDescriptor(MockDynamicSliceInstanceVegetationSystemComponent::CreateDescriptor()); + } + + protected: + AZStd::unique_ptr m_testHandler; + + private: AZ::JobManager* m_jobManager{ nullptr }; AZ::JobContext* m_jobContext{ nullptr }; + SetRestoreFileIOBaseRAII m_restoreFileIO; ::testing::NiceMock m_fileIOMock; }; @@ -276,7 +289,7 @@ namespace UnitTest Vegetation::DynamicSliceInstanceSpawner instanceSpawner2; // Give the second instance spawner a non-default asset reference. - CreateAndSetMockAsset(instanceSpawner2, AZ::Uuid::CreateRandom(), "test"); + m_testHandler->CreateAndSetMockAsset(instanceSpawner2, AZ::Uuid::CreateRandom(), "test"); // The test is written this way because only the == operator is overloaded. EXPECT_TRUE(!(instanceSpawner1 == instanceSpawner2)); @@ -292,14 +305,14 @@ namespace UnitTest EXPECT_TRUE(instanceSpawner.HasEmptyAssetReferences()); // This will test the asset load. - CreateAndSetMockAsset(instanceSpawner, AZ::Uuid::CreateRandom(), "test"); + m_testHandler->CreateAndSetMockAsset(instanceSpawner, AZ::Uuid::CreateRandom(), "test"); // Test the asset unload works too. - Vegetation::DescriptorNotificationBus::Handler::BusConnect(&instanceSpawner); + m_testHandler->Vegetation::DescriptorNotificationBus::Handler::BusConnect(&instanceSpawner); instanceSpawner.UnloadAssets(); EXPECT_FALSE(instanceSpawner.IsLoaded()); EXPECT_FALSE(instanceSpawner.IsSpawnable()); - Vegetation::DescriptorNotificationBus::Handler::BusDisconnect(); + m_testHandler->Vegetation::DescriptorNotificationBus::Handler::BusDisconnect(); } TEST_F(DynamicSliceInstanceSpawnerTests, CreateAndDestroyInstance) @@ -308,7 +321,7 @@ namespace UnitTest Vegetation::DynamicSliceInstanceSpawner instanceSpawner; - CreateAndSetMockAsset(instanceSpawner, AZ::Uuid::CreateRandom(), "test"); + m_testHandler->CreateAndSetMockAsset(instanceSpawner, AZ::Uuid::CreateRandom(), "test"); instanceSpawner.OnRegisterUniqueDescriptor(); diff --git a/Gems/Vegetation/Code/Tests/PrefabInstanceSpawnerTests.cpp b/Gems/Vegetation/Code/Tests/PrefabInstanceSpawnerTests.cpp index b0d88f2d63..3398dba20f 100644 --- a/Gems/Vegetation/Code/Tests/PrefabInstanceSpawnerTests.cpp +++ b/Gems/Vegetation/Code/Tests/PrefabInstanceSpawnerTests.cpp @@ -51,76 +51,21 @@ namespace UnitTest } }; - // To test prefab spawning, we need to mock up enough of the asset management system and the spawnable - // asset handling to pretend like we're loading/unloading spawnables successfully. - class PrefabInstanceSpawnerTests - : public VegetationComponentTests - , public UnitTest::SetRestoreFileIOBaseRAII - , public Vegetation::DescriptorNotificationBus::Handler + class PrefabInstanceHandlerAndCatalog + : public Vegetation::DescriptorNotificationBus::Handler , public AZ::Data::AssetCatalogRequestBus::Handler , public AZ::Data::AssetHandler , public AZ::Data::AssetCatalog { public: - PrefabInstanceSpawnerTests() - : UnitTest::SetRestoreFileIOBaseRAII(m_fileIOMock) + PrefabInstanceHandlerAndCatalog() { - AZ::IO::MockFileIOBase::InstallDefaultReturns(m_fileIOMock); - AzFramework::MockSpawnableEntitiesInterface::InstallDefaultReturns(m_spawnableEntitiesInterfaceMock); - } - - void RegisterComponentDescriptors() override - { - m_app.RegisterComponentDescriptor(MockPrefabInstanceVegetationSystemComponent::CreateDescriptor()); - } - - void SetUp() override - { - VegetationComponentTests::SetUp(); - - // Create a real Asset Mananger, and point to ourselves as the handler for Spawnable. - AZ::AllocatorInstance::Create(); - AZ::AllocatorInstance::Create(); - - - // Initialize the job manager with 1 thread for the AssetManager to use. - AZ::JobManagerDesc jobDesc; - AZ::JobManagerThreadDesc threadDesc; - jobDesc.m_workerThreads.push_back(threadDesc); - m_jobManager = aznew AZ::JobManager(jobDesc); - m_jobContext = aznew AZ::JobContext(*m_jobManager); - AZ::JobContext::SetGlobalContext(m_jobContext); - - AZ::Data::AssetManager::Descriptor descriptor; - AZ::Data::AssetManager::Create(descriptor); - AZ::Data::AssetManager::Instance().RegisterHandler(this, AZ::AzTypeInfo::Uuid()); - AZ::Data::AssetManager::Instance().RegisterCatalog(this, AZ::AzTypeInfo::Uuid()); - - // Intercept messages for finding assets by name. AZ::Data::AssetCatalogRequestBus::Handler::BusConnect(); } - void TearDown() override + ~PrefabInstanceHandlerAndCatalog() { - // Give the AssetManager a chance to fire off any lingering events and perform cleanup for any - // spawnable assets we loaded. - AZ::Data::AssetManager::Instance().DispatchEvents(); - - AZ::Data::AssetManager::Instance().UnregisterCatalog(this); - AZ::Data::AssetManager::Instance().UnregisterHandler(this); - AZ::Data::AssetCatalogRequestBus::Handler::BusDisconnect(); - - AZ::Data::AssetManager::Destroy(); - - AZ::JobContext::SetGlobalContext(nullptr); - delete m_jobContext; - delete m_jobManager; - - AZ::AllocatorInstance::Destroy(); - AZ::AllocatorInstance::Destroy(); - - VegetationComponentTests::TearDown(); } // Helper methods: @@ -227,9 +172,79 @@ namespace UnitTest AZStd::string m_assetPath; AZ::Data::AssetId m_assetId; int m_numOnLoadedCalls = 0; + }; + // To test Dynamic Slice spawning, we need to mock up enough of the asset management system and the dynamic slice + // asset handling to pretend like we're loading/unloading dynamic slices successfully. + class PrefabInstanceSpawnerTests + : public VegetationComponentTests + { + public: + PrefabInstanceSpawnerTests() + : m_restoreFileIO(m_fileIOMock) + { + AZ::IO::MockFileIOBase::InstallDefaultReturns(m_fileIOMock); + AzFramework::MockSpawnableEntitiesInterface::InstallDefaultReturns(m_spawnableEntitiesInterfaceMock); + } + + void SetUp() override + { + VegetationComponentTests::SetUp(); + + // Create a real Asset Mananger, and point to ourselves as the handler for DynamicSliceAsset. + AZ::AllocatorInstance::Create(); + AZ::AllocatorInstance::Create(); + + // Initialize the job manager with 1 thread for the AssetManager to use. + AZ::JobManagerDesc jobDesc; + AZ::JobManagerThreadDesc threadDesc; + jobDesc.m_workerThreads.push_back(threadDesc); + m_jobManager = aznew AZ::JobManager(jobDesc); + m_jobContext = aznew AZ::JobContext(*m_jobManager); + AZ::JobContext::SetGlobalContext(m_jobContext); + + AZ::Data::AssetManager::Descriptor descriptor; + AZ::Data::AssetManager::Create(descriptor); + m_testHandler = AZStd::make_unique(); + AZ::Data::AssetManager::Instance().RegisterHandler(m_testHandler.get(), AZ::AzTypeInfo::Uuid()); + AZ::Data::AssetManager::Instance().RegisterCatalog(m_testHandler.get(), AZ::AzTypeInfo::Uuid()); + + m_app.RegisterComponentDescriptor(AZ::SliceComponent::CreateDescriptor()); + } + + void TearDown() override + { + // Clear out the list of queued AssetBus Events before unregistering the AssetHandler + // to make sure pending references to Asset instances are cleared + AZ::Data::AssetManager::Instance().DispatchEvents(); + AZ::Data::AssetManager::Instance().UnregisterHandler(m_testHandler.get()); + AZ::Data::AssetManager::Instance().UnregisterCatalog(m_testHandler.get()); + AZ::Data::AssetManager::Destroy(); + + m_testHandler.reset(); + + AZ::JobContext::SetGlobalContext(nullptr); + delete m_jobContext; + delete m_jobManager; + + AZ::AllocatorInstance::Destroy(); + AZ::AllocatorInstance::Destroy(); + + VegetationComponentTests::TearDown(); + } + + void RegisterComponentDescriptors() override + { + m_app.RegisterComponentDescriptor(MockPrefabInstanceVegetationSystemComponent::CreateDescriptor()); + } + + protected: + AZStd::unique_ptr m_testHandler; + + private: AZ::JobManager* m_jobManager{ nullptr }; AZ::JobContext* m_jobContext{ nullptr }; + SetRestoreFileIOBaseRAII m_restoreFileIO; ::testing::NiceMock m_fileIOMock; ::testing::NiceMock m_spawnableEntitiesInterfaceMock; }; @@ -259,7 +274,7 @@ namespace UnitTest Vegetation::PrefabInstanceSpawner instanceSpawner2; // Give the second instance spawner a non-default asset reference. - CreateAndSetMockAsset(instanceSpawner2, AZ::Uuid::CreateRandom(), "test"); + m_testHandler->CreateAndSetMockAsset(instanceSpawner2, AZ::Uuid::CreateRandom(), "test"); // The test is written this way because only the == operator is overloaded. EXPECT_TRUE(!(instanceSpawner1 == instanceSpawner2)); @@ -275,14 +290,14 @@ namespace UnitTest EXPECT_TRUE(instanceSpawner.HasEmptyAssetReferences()); // This will test the asset load. - CreateAndSetMockAsset(instanceSpawner, AZ::Uuid::CreateRandom(), "test"); + m_testHandler->CreateAndSetMockAsset(instanceSpawner, AZ::Uuid::CreateRandom(), "test"); // Test the asset unload works too. - Vegetation::DescriptorNotificationBus::Handler::BusConnect(&instanceSpawner); + m_testHandler->Vegetation::DescriptorNotificationBus::Handler::BusConnect(&instanceSpawner); instanceSpawner.UnloadAssets(); EXPECT_FALSE(instanceSpawner.IsLoaded()); EXPECT_FALSE(instanceSpawner.IsSpawnable()); - Vegetation::DescriptorNotificationBus::Handler::BusDisconnect(); + m_testHandler->Vegetation::DescriptorNotificationBus::Handler::BusDisconnect(); } TEST_F(PrefabInstanceSpawnerTests, CreateAndDestroyInstance) @@ -291,7 +306,7 @@ namespace UnitTest Vegetation::PrefabInstanceSpawner instanceSpawner; - CreateAndSetMockAsset(instanceSpawner, AZ::Uuid::CreateRandom(), "test"); + m_testHandler->CreateAndSetMockAsset(instanceSpawner, AZ::Uuid::CreateRandom(), "test"); instanceSpawner.OnRegisterUniqueDescriptor(); diff --git a/Gems/Vegetation/Code/Tests/VegetationTest.h b/Gems/Vegetation/Code/Tests/VegetationTest.h index cf7c9b70a1..2ae8ac9cca 100644 --- a/Gems/Vegetation/Code/Tests/VegetationTest.h +++ b/Gems/Vegetation/Code/Tests/VegetationTest.h @@ -21,21 +21,34 @@ namespace UnitTest { class VegetationComponentTests - : public ::testing::Test + : public ScopedAllocatorSetupFixture { protected: + VegetationComponentTests() + : ScopedAllocatorSetupFixture( + []() { + AZ::SystemAllocator::Descriptor desc; + desc.m_heap.m_fixedMemoryBlocksByteSize[0] = 20 * 1024 * 1024; + desc.m_stackRecordLevels = 20; + return desc; + }() + ) + { + } + AZ::ComponentApplication m_app; virtual void RegisterComponentDescriptors() {} void SetUp() override { - AZ::ComponentApplication::Descriptor appDesc; - appDesc.m_memoryBlocksByteSize = 20 * 1024 * 1024; - appDesc.m_recordingMode = AZ::Debug::AllocationRecords::RECORD_NO_RECORDS; - appDesc.m_stackRecordLevels = 20; + if (AZ::Debug::AllocationRecords* records = AZ::AllocatorInstance::GetAllocator().GetRecords(); + records != nullptr) + { + records->SetMode(AZ::Debug::AllocationRecords::RECORD_NO_RECORDS); + } - m_app.Create(appDesc); + m_app.Create({}); RegisterComponentDescriptors(); }