Merge pull request #9 from GuusKemperman/thread-safe-assets

Thread-safe asset-management

Coral.vcxproj

@@ -325,6 +325,7 @@
     <ClCompile Include="Source\Systems\Rendering\ParticleRenderingSystem.cpp" />
     <ClCompile Include="Source\Systems\Rendering\StaticMeshRenderingSystem.cpp" />
     <ClCompile Include="Source\UnitTests\AssetHandleUnitTests.cpp" />
+    <ClCompile Include="Source\UnitTests\AssetManagerUnitTests.cpp" />
     <ClCompile Include="Source\Utilities\BVH.cpp" />
     <ClCompile Include="Source\Utilities\Events.cpp" />
     <ClCompile Include="Source\Utilities\Imgui\WorldDetailsPanel.cpp">

Games/ExampleGame/Source/Core/Main.cpp

@@ -3,18 +3,8 @@
 
 int main(int argc, char* argv[])
 {
-	std::cout << "Entered main - GameDir: " << GAME_DIR << std::endl;
+	CE::Engine engine{ argc, argv, GAME_DIR };
+	engine.Run("TestLevel");
 
-	try
-	{
-		CE::Engine engine{ argc, argv, GAME_DIR };
-		engine.Run("TestLevel");
-	}
-	catch ([[maybe_unused]] const std::exception& e)
-	{
-		std::cerr << "Fatal exception - " << e.what() << std::endl;
-		return 904;
-	}
-
 	return 0;
 }

Include/Assets/Core/AssetHandle.h

@@ -1,7 +1,6 @@
 #pragma once
 #include "AssetInternal.h"
 #include "Assets/Asset.h"
-#include "Meta/MetaTypeId.h"
 #include "Meta/MetaReflect.h"
 #include "Meta/MetaType.h"
 #include "Meta/MetaProps.h"
@@ -31,8 +30,6 @@ namespace CE
 
 		bool IsLoaded() const;
 
-		void Unload();
-
 		const std::optional<std::filesystem::path>& GetFileOfOrigin() const;
 
 		/**
@@ -44,6 +41,8 @@ namespace CE
 		uint32 GetNumberOfSoftReferences() const;
 
 	protected:
+		const Asset* GetAssetBase() const;
+
 		void AssureNotNull() const;
 
 		bool IsA(TypeId type) const;
@@ -118,7 +117,7 @@ namespace CE
 		AssetHandle& operator=(const AssetHandle<O>& other);
 
 		template<typename O, std::enable_if_t<std::is_convertible_v<O*, T*>, bool> = true>
-		explicit operator WeakAssetHandle<O>() const;
+		operator WeakAssetHandle<O>() const;
 
 		const T& operator*() const;
 		const T* operator->() const;
@@ -156,7 +155,7 @@ namespace CE
 		WeakAssetHandle& operator=(const WeakAssetHandle<O>& other);
 
 		template<typename O, std::enable_if_t<std::is_convertible_v<O*, T*>, bool> = true>
-		explicit operator AssetHandle<O>() const;
+		operator AssetHandle<O>() const;
 
 	private:
 		template<typename U, typename O, std::enable_if_t<std::is_convertible_v<U*, O*>, bool>>
@@ -169,6 +168,12 @@ namespace CE
 		static MetaType Reflect();
 	};
 
+	template<typename T>
+	AssetHandle(WeakAssetHandle<T>) -> AssetHandle<T>;
+
+	template<typename T>
+	WeakAssetHandle(AssetHandle<T>) -> WeakAssetHandle<T>;
+
 	template <Internal::AssetInternal::RefCountType IndexOfCounter>
 	AssetHandleRefCounter<IndexOfCounter>::AssetHandleRefCounter() = default;
 
@@ -295,25 +300,12 @@ namespace CE
 	template <typename T>
 	const T* AssetHandle<T>::Get() const
 	{
-		if (mAssetInternal == nullptr)
-		{
-			return nullptr;
-		}
-
-		mAssetInternal->mHasBeenDereferencedSinceGarbageCollect = true;
-
-		if (!IsLoaded())
-		{
-			mAssetInternal->Load();
-		}
-		ASSERT(IsLoaded());
-
 		// We do a reinterpret_cast,
 		// otherwise if we use a static_cast
 		// the user is forced to include the
 		// entire header of where the asset is
 		// located.
-		return reinterpret_cast<const T*>(mAssetInternal->mAsset.get());
+		return reinterpret_cast<const T*>(GetAssetBase());
 	}
 
 	template <typename T, typename O, std::enable_if_t<std::is_convertible_v<T*, O*>, bool>>

Include/Assets/Core/AssetInternal.h

@@ -16,18 +16,17 @@ namespace CE::Internal
 	{
 		AssetInternal(AssetFileMetaData&& metaData, const std::optional<std::filesystem::path>& path);
 
-		void Load();
-		void UnLoad();
+		Asset* TryGetLoadedAsset();
 
 		enum class RefCountType : bool { Strong, Weak };
 
-		std::array<uint32, 2> mRefCounters{};
-
 		std::unique_ptr<Asset, InPlaceDeleter<Asset, true>> mAsset{};
 
-		AssetFileMetaData mMetaData;
+		std::array<std::atomic<uint32>, 2> mRefCounters{};
+		std::atomic<bool> mHasBeenDereferencedSinceGarbageCollect{};
+		std::mutex mAccessMutex{};
 
-		bool mHasBeenDereferencedSinceGarbageCollect{};
+		AssetFileMetaData mMetaData;
 
 		// The .asset file. Is only nullopt if this
 		// asset was generated at runtime, and no path
@@ -36,6 +35,5 @@ namespace CE::Internal
 
 		// The .rename files that redirect to this asset.
 		std::vector<std::filesystem::path> mOldNames{};
-
 	};
 }

Include/Assets/StaticMesh.h

@@ -24,7 +24,7 @@ namespace CE
         AABB3D GetBoundingBox() const { return mBoundingBox; }
 #endif
 
-        const StaticMeshPlatformImpl* GetPlatformImpl() const { return mImpl.get(); }
+        const std::shared_ptr<StaticMeshPlatformImpl>& GetPlatformImpl() const { return mImpl; }
 
     private:
         friend ReflectAccess;

Include/Assets/Texture.h

@@ -25,7 +25,7 @@ namespace CE
 
 		glm::ivec2 GetSize() const { return mSize; }
 
-		TexturePlatformImpl* GetPlatformImpl() const { return mImpl.get(); }
+		const std::shared_ptr<TexturePlatformImpl>& GetPlatformImpl() const { return mImpl; }
 
 		static AssetHandle<Texture> TryGetDefaultTexture();
 

Include/Core/AssetManager.h

@@ -76,14 +76,29 @@ namespace CE
 		WeakAssetHandle<T> TryGetWeakAsset(Name key);
 
 		/*
-		If you want to save on memory, it is recommended to call this every frame
-		or at an interval, at the cost that this will lead to assets 
-		being loaded/unloaded more frequently.
+		
+
 
-		Assets generated at runtime are not unloaded automatically, because
-		they cannot be loaded back in by the asset manager.
 		*/
-		void UnloadAllUnusedAssets();
+		/**
+		 * \brief Unloads all assets that are considered unused.
+		 *
+		 * If you want to save on memory, it is recommended to
+		 * call this at an interval, at the cost that this will
+		 * lead to assets being loaded/unloaded more frequently.
+		 *
+		 * An asset may be considered based on a variety of factors,
+		 * such as the number of references or if it was recently
+		 * loaded.
+		 *
+		 * Assets generated at runtime are not unloaded automatically,
+		 * because they cannot be loaded back in by the asset manager.
+		 *
+		 * \param shouldSkipRecentlyDereferenced Assets that were
+		 * dereferenced since the last time this function was called
+		 * are always considered in use.
+		 */
+		void UnloadAllUnusedAssets(bool shouldSkipRecentlyDereferenced);
 
 		template<typename AssetType>
 		class EachAssetIt
@@ -193,7 +208,6 @@ namespace CE
 		std::unordered_map<Name::HashType, std::reference_wrapper<Internal::AssetInternal>> mLookUp{};
 
 		Internal::AssetInternal* TryGetAssetInternal(Name key, TypeId typeId);
-		Internal::AssetInternal* TryGetLoadedAssetInternal(Name key, TypeId typeId);
 
 		Internal::AssetInternal* TryConstruct(const std::filesystem::path& path);
 		Internal::AssetInternal* TryConstruct(const std::optional<std::filesystem::path>& path, AssetFileMetaData metaData);

Include/Core/VirtualMachine.h

@@ -91,8 +91,12 @@ namespace CE
 		static constexpr uint32 sMaxNumOfNodesToExecutePerFunctionBeforeGivingUp = 10'000;
 		static constexpr uint32 sMaxStackSize = 1 << 16;
 
-		std::array<char, sMaxStackSize> mStack{};
-		char* mStackPtr = &mStack[0];
+		struct Stack
+		{
+			std::array<char, sMaxStackSize> mData;
+			char* mStackPtr = mData.data();
+		};
+		static Stack& GetStack();
 
 		std::vector<ScriptError> mErrorsFromLastCompilation{};
 		bool mIsCompiled{};

Include/Utilities/Reflect/ReflectAssetType.h

@@ -10,23 +10,6 @@
 
 namespace CE
 {
-	namespace Props
-	{
-		/*
-		Use on:
-			Assets
-
-		Description:
-			Can be used to indicate that assets of this type will never have dependencies
-			on other assets. This means they do not need to be unloaded when the editor
-			refreshes, and can lead to performance benefits.
-
-		Example:
-			type.GetProperties().Add(Props::sCannotReferenceOtherAssetsTag)
-		*/
-		static constexpr std::string_view sCannotReferenceOtherAssetsTag = "sCannotReferenceOtherAssetsTag";
-	}
-
 	// Makes sure the type receives all the functionality that an Asset requires.
 	template<typename T>
 	void ReflectAssetType([[maybe_unused]] MetaType& type)

Source/Assets/Core/AssetHandle.cpp

@@ -23,6 +23,11 @@ CE::AssetHandleBase::AssetHandleBase(AssetHandleBase&& other) noexcept :
 
 CE::AssetHandleBase& CE::AssetHandleBase::operator=(AssetHandleBase&& other) noexcept
 {
+	if (this == &other)
+	{
+		return *this;
+	}
+
 	mAssetInternal = other.mAssetInternal;
 	other.mAssetInternal = nullptr;
 	return *this;
@@ -64,14 +69,6 @@ bool CE::AssetHandleBase::IsLoaded() const
 	return mAssetInternal != nullptr && mAssetInternal->mAsset != nullptr;
 }
 
-void CE::AssetHandleBase::Unload()
-{
-	if (IsLoaded())
-	{
-		mAssetInternal->UnLoad();
-	}
-}
-
 const std::optional<std::filesystem::path>& CE::AssetHandleBase::GetFileOfOrigin() const
 {
 	AssureNotNull();
@@ -113,6 +110,17 @@ uint32 CE::AssetHandleBase::GetNumberOfSoftReferences() const
 	return mAssetInternal->mRefCounters[static_cast<int>(Internal::AssetInternal::RefCountType::Weak)];
 }
 
+const CE::Asset* CE::AssetHandleBase::GetAssetBase() const
+{
+	if (mAssetInternal == nullptr)
+	{
+		return nullptr;
+	}
+
+	mAssetInternal->mHasBeenDereferencedSinceGarbageCollect = true;
+	return mAssetInternal->TryGetLoadedAsset();
+}
+
 void CE::AssetHandleBase::AssureNotNull() const
 {
 	ASSERT_LOG(*this != nullptr, "Attempted to dereference null handle");

Source/Assets/Core/AssetInternal.cpp

@@ -12,21 +12,20 @@ CE::Internal::AssetInternal::AssetInternal(AssetFileMetaData&& metaData, const s
 {
 }
 
-void CE::Internal::AssetInternal::Load()
+CE::Asset* CE::Internal::AssetInternal::TryGetLoadedAsset()
 {
-	LOG(LogAssets, Verbose, "Loading {}", mMetaData.GetName());
+	std::unique_lock lock{ mAccessMutex };
 
 	if (mAsset != nullptr)
 	{
-		LOG(LogAssets, Warning, "Attempting to load {} twice", mMetaData.GetName());
-		return;
+		return mAsset.get();
 	}
 
 	if (!mFileOfOrigin.has_value())
 	{
 		LOG(LogAssets, Error, "Attempted to load {}, but this asset was generated at runtime and should not have been unloaded to begin with.",
 			mMetaData.GetName());
-		return;
+		return nullptr;
 	}
 
 	std::optional<AssetLoadInfo> loadInfo = AssetLoadInfo::LoadFromFile(*mFileOfOrigin);
@@ -35,7 +34,7 @@ void CE::Internal::AssetInternal::Load()
 	{
 		LOG(LogAssets, Error, "Asset {} could not be loaded, the metadata failed to load.",
 			mMetaData.GetName());
-		return;
+		return nullptr;
 	}
 
 	ASSERT(loadInfo.has_value());
@@ -47,19 +46,8 @@ void CE::Internal::AssetInternal::Load()
 		LOG(LogAssets, Error, "Asset of type {} could not be constructed. Does it have a constructor that takes a LoadInfo&, and was this constructor reflected in your reflect function? {}",
 			mMetaData.GetClass().GetName(),
 			constructResult.Error());
-		return;
+		return nullptr;
 	}
 	mAsset = MakeUnique<Asset>(std::move(constructResult.GetReturnValue()));
-}
-
-void CE::Internal::AssetInternal::UnLoad()
-{
-	if (mAsset != nullptr)
-	{
-		mAsset.reset();
-	}
-	else
-	{
-		LOG(LogAssets, Verbose, "Asset {} is already unloaded", mMetaData.GetName());
-	}
+	return mAsset.get();
 }

Source/Assets/Font.cpp

@@ -67,7 +67,6 @@ CE::Font::Font(AssetLoadInfo& loadInfo) :
 CE::MetaType CE::Font::Reflect()
 {
     MetaType type = MetaType{ MetaType::T<Font>{}, "Font", MetaType::Base<Asset>{}, MetaType::Ctor<AssetLoadInfo&>{}, MetaType::Ctor<std::string_view>{} };
-    type.GetProperties().Add(Props::sCannotReferenceOtherAssetsTag);
     ReflectAssetType<Font>(type);
     return type;
 }

Source/Assets/Sound.cpp

@@ -68,7 +68,6 @@ FMOD::Channel* CE::Sound::Play(Audio::Group group) const
 CE::MetaType CE::Sound::Reflect()
 {
     MetaType type = MetaType{ MetaType::T<Sound>{}, "Sound", MetaType::Base<Asset>{}, MetaType::Ctor<AssetLoadInfo&>{}, MetaType::Ctor<std::string_view>{} };
-    type.GetProperties().Add(Props::sCannotReferenceOtherAssetsTag);
     ReflectAssetType<Sound>(type);
     return type;
 }

Source/Assets/StaticMesh.cpp

@@ -5,6 +5,7 @@
 
 #include "Assets/Core/AssetLoadInfo.h"
 #include "Assets/Core/AssetSaveInfo.h"
+#include "Core/Device.h"
 #include "Core/Renderer.h"
 #include "Utilities/ClassVersion.h"
 #include "Utilities/StringFunctions.h"
@@ -81,7 +82,6 @@ CE::StaticMesh::StaticMesh(AssetLoadInfo& loadInfo) :
 CE::MetaType CE::StaticMesh::Reflect()
 {
 	MetaType type = MetaType{ MetaType::T<StaticMesh>{}, "StaticMesh", MetaType::Base<Asset>{}, MetaType::Ctor<AssetLoadInfo&>{}, MetaType::Ctor<std::string_view>{} };
-	type.GetProperties().Add(Props::sCannotReferenceOtherAssetsTag);
 
 	SetClassVersion(type, 1);
 

Source/Assets/Texture.cpp

@@ -49,7 +49,6 @@ CE::Texture::Texture(AssetLoadInfo& loadInfo) :
 CE::MetaType CE::Texture::Reflect()
 {
 	MetaType type = MetaType{ MetaType::T<Texture>{}, "Texture", MetaType::Base<Asset>{}, MetaType::Ctor<AssetLoadInfo&>{}, MetaType::Ctor<std::string_view>{} };
-	type.GetProperties().Add(Props::sCannotReferenceOtherAssetsTag);
 	ReflectAssetType<Texture>(type);
 	return type;
 }