From 3e363c82754057281adf04c650ad610d362266d2 Mon Sep 17 00:00:00 2001 From: Wojtek Figat Date: Thu, 4 Sep 2025 14:48:52 +0200 Subject: [PATCH] Remove `ConcurrentSystemLocker` and use `ReadWriteLock` instead of better threading synchronization --- Source/Engine/Animations/Animations.cpp | 10 +-- Source/Engine/Animations/Animations.h | 3 +- Source/Engine/Content/Asset.h | 1 - Source/Engine/Content/Assets/Animation.cpp | 4 +- .../Engine/Content/Assets/AnimationGraph.cpp | 8 +- .../Content/Assets/AnimationGraphFunction.cpp | 6 +- Source/Engine/Level/Scene/SceneRendering.cpp | 21 +++--- Source/Engine/Level/Scene/SceneRendering.h | 4 +- Source/Engine/Particles/ParticleEmitter.cpp | 13 ++-- .../Particles/ParticleEmitterFunction.cpp | 6 +- Source/Engine/Particles/Particles.cpp | 14 ++-- Source/Engine/Particles/Particles.h | 5 +- .../Renderer/GlobalSignDistanceFieldPass.cpp | 16 ++-- .../Threading/ConcurrentSystemLocker.cpp | 75 ------------------- .../Engine/Threading/ConcurrentSystemLocker.h | 47 ------------ Source/Engine/Threading/Threading.h | 68 +++++++++++++---- 16 files changed, 107 insertions(+), 194 deletions(-) delete mode 100644 Source/Engine/Threading/ConcurrentSystemLocker.cpp delete mode 100644 Source/Engine/Threading/ConcurrentSystemLocker.h diff --git a/Source/Engine/Animations/Animations.cpp b/Source/Engine/Animations/Animations.cpp index d2708edad..f20f41a58 100644 --- a/Source/Engine/Animations/Animations.cpp +++ b/Source/Engine/Animations/Animations.cpp @@ -53,7 +53,7 @@ namespace AnimationsService AnimationManagerInstance; TaskGraphSystem* Animations::System = nullptr; -ConcurrentSystemLocker Animations::SystemLocker; +ReadWriteLock Animations::SystemLocker; #if USE_EDITOR Delegate Animations::DebugFlow; #endif @@ -124,7 +124,7 @@ void AnimationsSystem::Execute(TaskGraph* graph) Active = true; // Ensure no animation assets can be reloaded/modified during async update - Animations::SystemLocker.Begin(false); + Animations::SystemLocker.ReadLock(); // Setup data for async update const auto& tickData = Time::Update; @@ -165,18 +165,18 @@ void AnimationsSystem::PostExecute(TaskGraph* graph) // Cleanup AnimationManagerInstance.UpdateList.Clear(); - Animations::SystemLocker.End(false); + Animations::SystemLocker.ReadUnlock(); Active = false; } void Animations::AddToUpdate(AnimatedModel* obj) { - ConcurrentSystemLocker::WriteScope lock(SystemLocker, true); + ScopeWriteLock lock(SystemLocker); AnimationManagerInstance.UpdateList.Add(obj); } void Animations::RemoveFromUpdate(AnimatedModel* obj) { - ConcurrentSystemLocker::WriteScope lock(SystemLocker, true); + ScopeWriteLock lock(SystemLocker); AnimationManagerInstance.UpdateList.Remove(obj); } diff --git a/Source/Engine/Animations/Animations.h b/Source/Engine/Animations/Animations.h index ac30640dc..f8aa28965 100644 --- a/Source/Engine/Animations/Animations.h +++ b/Source/Engine/Animations/Animations.h @@ -4,7 +4,6 @@ #include "Engine/Scripting/ScriptingType.h" #include "Engine/Core/Delegate.h" -#include "Engine/Threading/ConcurrentSystemLocker.h" class TaskGraphSystem; class AnimatedModel; @@ -23,7 +22,7 @@ API_CLASS(Static) class FLAXENGINE_API Animations API_FIELD(ReadOnly) static TaskGraphSystem* System; // Data access locker for animations data. - static ConcurrentSystemLocker SystemLocker; + static ReadWriteLock SystemLocker; #if USE_EDITOR // Data wrapper for the debug flow information. diff --git a/Source/Engine/Content/Asset.h b/Source/Engine/Content/Asset.h index 32f2a5e31..e6607e62c 100644 --- a/Source/Engine/Content/Asset.h +++ b/Source/Engine/Content/Asset.h @@ -7,7 +7,6 @@ #include "Engine/Core/Types/String.h" #include "Engine/Platform/CriticalSection.h" #include "Engine/Scripting/ScriptingObject.h" -#include "Engine/Threading/ConcurrentSystemLocker.h" #include "Config.h" #include "Types.h" diff --git a/Source/Engine/Content/Assets/Animation.cpp b/Source/Engine/Content/Assets/Animation.cpp index 93397397a..015f09e4e 100644 --- a/Source/Engine/Content/Assets/Animation.cpp +++ b/Source/Engine/Content/Assets/Animation.cpp @@ -600,7 +600,7 @@ void Animation::OnScriptingDispose() Asset::LoadResult Animation::load() { PROFILE_MEM(AnimationsData); - ConcurrentSystemLocker::WriteScope systemScope(Animations::SystemLocker); + ScopeWriteLock systemScope(Animations::SystemLocker); // Get stream with animations data const auto dataChunk = GetChunk(0); @@ -732,7 +732,7 @@ Asset::LoadResult Animation::load() void Animation::unload(bool isReloading) { - ConcurrentSystemLocker::WriteScope systemScope(Animations::SystemLocker); + ScopeWriteLock systemScope(Animations::SystemLocker); #if USE_EDITOR if (_registeredForScriptingReload) { diff --git a/Source/Engine/Content/Assets/AnimationGraph.cpp b/Source/Engine/Content/Assets/AnimationGraph.cpp index acab48b2f..ad6353196 100644 --- a/Source/Engine/Content/Assets/AnimationGraph.cpp +++ b/Source/Engine/Content/Assets/AnimationGraph.cpp @@ -27,7 +27,7 @@ AnimationGraph::AnimationGraph(const SpawnParams& params, const AssetInfo* info) Asset::LoadResult AnimationGraph::load() { PROFILE_MEM(AnimationsData); - ConcurrentSystemLocker::WriteScope systemScope(Animations::SystemLocker); + ScopeWriteLock systemScope(Animations::SystemLocker); // Get stream with graph data const auto surfaceChunk = GetChunk(0); @@ -53,7 +53,7 @@ Asset::LoadResult AnimationGraph::load() void AnimationGraph::unload(bool isReloading) { - ConcurrentSystemLocker::WriteScope systemScope(Animations::SystemLocker); + ScopeWriteLock systemScope(Animations::SystemLocker); Graph.Clear(); } @@ -86,7 +86,7 @@ bool AnimationGraph::InitAsAnimation(SkinnedModel* baseModel, Animation* anim, b return true; } PROFILE_MEM(AnimationsData); - ConcurrentSystemLocker::WriteScope systemScope(Animations::SystemLocker); + ScopeWriteLock systemScope(Animations::SystemLocker); // Create Graph data MemoryWriteStream writeStream(512); @@ -172,7 +172,7 @@ bool AnimationGraph::SaveSurface(const BytesContainer& data) { if (OnCheckSave()) return true; - ConcurrentSystemLocker::WriteScope systemScope(Animations::SystemLocker); + ScopeWriteLock systemScope(Animations::SystemLocker); ScopeLock lock(Locker); if (IsVirtual()) diff --git a/Source/Engine/Content/Assets/AnimationGraphFunction.cpp b/Source/Engine/Content/Assets/AnimationGraphFunction.cpp index 3e8ce62e8..812a8f090 100644 --- a/Source/Engine/Content/Assets/AnimationGraphFunction.cpp +++ b/Source/Engine/Content/Assets/AnimationGraphFunction.cpp @@ -22,7 +22,7 @@ AnimationGraphFunction::AnimationGraphFunction(const SpawnParams& params, const Asset::LoadResult AnimationGraphFunction::load() { PROFILE_MEM(AnimationsData); - ConcurrentSystemLocker::WriteScope systemScope(Animations::SystemLocker); + ScopeWriteLock systemScope(Animations::SystemLocker); // Get graph data from chunk const auto surfaceChunk = GetChunk(0); @@ -49,7 +49,7 @@ Asset::LoadResult AnimationGraphFunction::load() void AnimationGraphFunction::unload(bool isReloading) { - ConcurrentSystemLocker::WriteScope systemScope(Animations::SystemLocker); + ScopeWriteLock systemScope(Animations::SystemLocker); GraphData.Release(); Inputs.Clear(); Outputs.Clear(); @@ -98,7 +98,7 @@ bool AnimationGraphFunction::SaveSurface(const BytesContainer& data) const { if (OnCheckSave()) return true; - ConcurrentSystemLocker::WriteScope systemScope(Animations::SystemLocker); + ScopeWriteLock systemScope(Animations::SystemLocker); ScopeLock lock(Locker); // Set Visject Surface data diff --git a/Source/Engine/Level/Scene/SceneRendering.cpp b/Source/Engine/Level/Scene/SceneRendering.cpp index 1fffa79c8..fac756af8 100644 --- a/Source/Engine/Level/Scene/SceneRendering.cpp +++ b/Source/Engine/Level/Scene/SceneRendering.cpp @@ -19,7 +19,7 @@ #define CHECK_SCENE_EDIT_ACCESS() #else #define CHECK_SCENE_EDIT_ACCESS() \ - if (Locker.HasLock(false) && IsInMainThread() && GPUDevice::Instance && GPUDevice::Instance->IsRendering()) \ + if (_isRendering && IsInMainThread() && GPUDevice::Instance && GPUDevice::Instance->IsRendering()) \ { \ LOG(Error, "Adding/removing actors during rendering is not supported ({}, '{}').", a->ToString(), a->GetNamePath()); \ return; \ @@ -58,20 +58,21 @@ FORCE_INLINE bool FrustumsListCull(const BoundingSphere& bounds, const ArrayScenes.Add(this); - - // Add additional lock during scene rendering (prevents any Actors cache modifications on content streaming threads - eg. when model residency changes) - Locker.Begin(false); } else if (category == PostRender) { // Release additional lock - Locker.End(false); + _isRendering = false; + Locker.ReadUnlock(); } auto& view = renderContextBatch.GetMainContext().View; auto& list = Actors[(int32)category]; @@ -142,7 +143,7 @@ void SceneRendering::CollectPostFxVolumes(RenderContext& renderContext) void SceneRendering::Clear() { - ConcurrentSystemLocker::WriteScope lock(Locker, true); + ScopeWriteLock lock(Locker); for (auto* listener : _listeners) { listener->OnSceneRenderingClear(this); @@ -165,7 +166,7 @@ void SceneRendering::AddActor(Actor* a, int32& key) PROFILE_MEM(Graphics); CHECK_SCENE_EDIT_ACCESS(); const int32 category = a->_drawCategory; - ConcurrentSystemLocker::WriteScope lock(Locker, true); + ScopeWriteLock lock(Locker); auto& list = Actors[category]; if (FreeActors[category].HasItems()) { @@ -190,7 +191,7 @@ void SceneRendering::AddActor(Actor* a, int32& key) void SceneRendering::UpdateActor(Actor* a, int32& key, ISceneRenderingListener::UpdateFlags flags) { const int32 category = a->_drawCategory; - ConcurrentSystemLocker::ReadScope lock(Locker); // Read-access only as list doesn't get resized (like Add/Remove do) so allow updating actors from different threads at once + ScopeReadLock lock(Locker); // Read-access only as list doesn't get resized (like Add/Remove do) so allow updating actors from different threads at once auto& list = Actors[category]; if (list.Count() <= key || key < 0) // Ignore invalid key softly return; @@ -210,7 +211,7 @@ void SceneRendering::RemoveActor(Actor* a, int32& key) { CHECK_SCENE_EDIT_ACCESS(); const int32 category = a->_drawCategory; - ConcurrentSystemLocker::WriteScope lock(Locker, true); + ScopeWriteLock lock(Locker); auto& list = Actors[category]; if (list.Count() > key || key < 0) // Ignore invalid key softly (eg. list after batch clear during scene unload) { diff --git a/Source/Engine/Level/Scene/SceneRendering.h b/Source/Engine/Level/Scene/SceneRendering.h index 293927ea8..c11c94de8 100644 --- a/Source/Engine/Level/Scene/SceneRendering.h +++ b/Source/Engine/Level/Scene/SceneRendering.h @@ -7,7 +7,6 @@ #include "Engine/Core/Math/BoundingSphere.h" #include "Engine/Core/Math/BoundingFrustum.h" #include "Engine/Level/Actor.h" -#include "Engine/Threading/ConcurrentSystemLocker.h" class SceneRenderTask; class SceneRendering; @@ -102,9 +101,10 @@ public: Array Actors[MAX]; Array FreeActors[MAX]; Array PostFxProviders; - ConcurrentSystemLocker Locker; + ReadWriteLock Locker; private: + bool _isRendering = false; #if USE_EDITOR Array PhysicsDebug; Array LightsDebug; diff --git a/Source/Engine/Particles/ParticleEmitter.cpp b/Source/Engine/Particles/ParticleEmitter.cpp index 9991692c2..80738e65d 100644 --- a/Source/Engine/Particles/ParticleEmitter.cpp +++ b/Source/Engine/Particles/ParticleEmitter.cpp @@ -106,7 +106,7 @@ namespace Asset::LoadResult ParticleEmitter::load() { PROFILE_MEM(Particles); - ConcurrentSystemLocker::WriteScope systemScope(Particles::SystemLocker); + ScopeWriteLock systemScope(Particles::SystemLocker); // Load the graph const auto surfaceChunk = GetChunk(SHADER_FILE_CHUNK_VISJECT_SURFACE); @@ -330,6 +330,7 @@ Asset::LoadResult ParticleEmitter::load() // Wait for resources used by the emitter to be loaded // eg. texture used to place particles on spawn needs to be available + // Free Particles::SystemLocker when waiting on asset load to prevent lock-contention. bool waitForAsset = false; for (const auto& node : Graph.Nodes) { @@ -341,7 +342,7 @@ Asset::LoadResult ParticleEmitter::load() if (!waitForAsset) { waitForAsset = true; - Particles::SystemLocker.End(true); + Particles::SystemLocker.WriteUnlock(); } WaitForAsset(texture); } @@ -354,20 +355,20 @@ Asset::LoadResult ParticleEmitter::load() if (!waitForAsset) { waitForAsset = true; - Particles::SystemLocker.End(true); + Particles::SystemLocker.WriteUnlock(); } WaitForAsset((Asset*)parameter.Value); } } if (waitForAsset) - Particles::SystemLocker.Begin(true); + Particles::SystemLocker.WriteLock(); return LoadResult::Ok; } void ParticleEmitter::unload(bool isReloading) { - ConcurrentSystemLocker::WriteScope systemScope(Particles::SystemLocker); + ScopeWriteLock systemScope(Particles::SystemLocker); #if COMPILE_WITH_SHADER_COMPILER UnregisterForShaderReloads(this); #endif @@ -458,7 +459,7 @@ bool ParticleEmitter::SaveSurface(const BytesContainer& data) { if (OnCheckSave()) return true; - ConcurrentSystemLocker::WriteScope systemScope(Particles::SystemLocker); + ScopeWriteLock systemScope(Particles::SystemLocker); ScopeLock lock(Locker); // Release all chunks diff --git a/Source/Engine/Particles/ParticleEmitterFunction.cpp b/Source/Engine/Particles/ParticleEmitterFunction.cpp index f8aa5c62a..37e879172 100644 --- a/Source/Engine/Particles/ParticleEmitterFunction.cpp +++ b/Source/Engine/Particles/ParticleEmitterFunction.cpp @@ -43,7 +43,7 @@ ParticleEmitterFunction::ParticleEmitterFunction(const SpawnParams& params, cons Asset::LoadResult ParticleEmitterFunction::load() { PROFILE_MEM(Particles); - ConcurrentSystemLocker::WriteScope systemScope(Particles::SystemLocker); + ScopeWriteLock systemScope(Particles::SystemLocker); // Load graph const auto surfaceChunk = GetChunk(0); @@ -96,7 +96,7 @@ Asset::LoadResult ParticleEmitterFunction::load() void ParticleEmitterFunction::unload(bool isReloading) { - ConcurrentSystemLocker::WriteScope systemScope(Particles::SystemLocker); + ScopeWriteLock systemScope(Particles::SystemLocker); Graph.Clear(); #if COMPILE_WITH_PARTICLE_GPU_GRAPH GraphGPU.Clear(); @@ -189,7 +189,7 @@ bool ParticleEmitterFunction::SaveSurface(const BytesContainer& data) const { if (OnCheckSave()) return true; - ConcurrentSystemLocker::WriteScope systemScope(Particles::SystemLocker); + ScopeWriteLock systemScope(Particles::SystemLocker); ScopeLock lock(Locker); // Set Visject Surface data diff --git a/Source/Engine/Particles/Particles.cpp b/Source/Engine/Particles/Particles.cpp index 88fa2cd88..ac02e46d3 100644 --- a/Source/Engine/Particles/Particles.cpp +++ b/Source/Engine/Particles/Particles.cpp @@ -134,7 +134,7 @@ namespace ParticleManagerImpl using namespace ParticleManagerImpl; TaskGraphSystem* Particles::System = nullptr; -ConcurrentSystemLocker Particles::SystemLocker; +ReadWriteLock Particles::SystemLocker; bool Particles::EnableParticleBufferPooling = true; float Particles::ParticleBufferRecycleTimeout = 10.0f; @@ -680,7 +680,7 @@ void CleanupGPUParticlesSorting() void DrawEmittersGPU(RenderContextBatch& renderContextBatch) { PROFILE_GPU_CPU_NAMED("DrawEmittersGPU"); - ConcurrentSystemLocker::ReadScope systemScope(Particles::SystemLocker); + ScopeReadLock systemScope(Particles::SystemLocker); GPUContext* context = GPUDevice::Instance->GetMainContext(); // Count draws and sorting passes needed for resources allocation @@ -1135,7 +1135,7 @@ void Particles::DrawParticles(RenderContextBatch& renderContextBatch, ParticleEf viewsDrawModes &= effect->DrawModes; // Setup - ConcurrentSystemLocker::ReadScope systemScope(SystemLocker); + ScopeReadLock systemScope(SystemLocker); Matrix worlds[2]; Matrix::Translation(-viewOrigin, worlds[0]); // World renderContextBatch.GetMainContext().View.GetWorldMatrix(effect->GetTransform(), worlds[1]); // Local @@ -1277,7 +1277,7 @@ void Particles::DrawParticles(RenderContextBatch& renderContextBatch, ParticleEf void Particles::DebugDraw(ParticleEffect* effect) { PROFILE_CPU_NAMED("Particles.DrawDebug"); - ConcurrentSystemLocker::ReadScope systemScope(SystemLocker); + ScopeReadLock systemScope(SystemLocker); // Draw all emitters for (auto& emitterData : effect->Instance.Emitters) @@ -1304,7 +1304,7 @@ void UpdateGPU(RenderTask* task, GPUContext* context) PROFILE_CPU_NAMED("GPUParticles"); PROFILE_GPU("GPU Particles"); PROFILE_MEM(Particles); - ConcurrentSystemLocker::ReadScope systemScope(Particles::SystemLocker); + ScopeReadLock systemScope(Particles::SystemLocker); // Collect valid emitter tracks to update struct GPUSim @@ -1728,7 +1728,7 @@ void ParticlesSystem::Execute(TaskGraph* graph) Active = true; // Ensure no particle assets can be reloaded/modified during async update - Particles::SystemLocker.Begin(false); + Particles::SystemLocker.ReadLock(); // Setup data for async update const auto& tickData = Time::Update; @@ -1751,7 +1751,7 @@ void ParticlesSystem::PostExecute(TaskGraph* graph) PROFILE_MEM(Particles); // Cleanup - Particles::SystemLocker.End(false); + Particles::SystemLocker.ReadUnlock(); Active = false; UpdateList.Clear(); diff --git a/Source/Engine/Particles/Particles.h b/Source/Engine/Particles/Particles.h index 38ec39d2d..2007a3d8d 100644 --- a/Source/Engine/Particles/Particles.h +++ b/Source/Engine/Particles/Particles.h @@ -3,7 +3,6 @@ #pragma once #include "Engine/Scripting/ScriptingType.h" -#include "Engine/Threading/ConcurrentSystemLocker.h" class TaskGraphSystem; struct RenderContextBatch; @@ -28,8 +27,8 @@ API_CLASS(Static) class FLAXENGINE_API Particles /// API_FIELD(ReadOnly) static TaskGraphSystem* System; - // Data access locker for animations data. - static ConcurrentSystemLocker SystemLocker; + // Data access locker for particles data. + static ReadWriteLock SystemLocker; public: /// diff --git a/Source/Engine/Renderer/GlobalSignDistanceFieldPass.cpp b/Source/Engine/Renderer/GlobalSignDistanceFieldPass.cpp index 85502cc97..1240f148c 100644 --- a/Source/Engine/Renderer/GlobalSignDistanceFieldPass.cpp +++ b/Source/Engine/Renderer/GlobalSignDistanceFieldPass.cpp @@ -198,7 +198,7 @@ public: GPUTexture* Texture = nullptr; GPUTexture* TextureMip = nullptr; Vector3 Origin = Vector3::Zero; - ConcurrentSystemLocker Locker; + ReadWriteLock Locker; Array> Cascades; HashSet ObjectTypes; HashSet SDFTextures; @@ -238,7 +238,7 @@ public: OnSDFTextureDeleted(texture); // Clear static chunks cache - ConcurrentSystemLocker::WriteScope lock(Locker, true); + ScopeWriteLock lock(Locker); for (auto& cascade : Cascades) cascade.StaticChunks.Clear(); } @@ -398,7 +398,7 @@ public: { if (GLOBAL_SDF_ACTOR_IS_STATIC(a) && ObjectTypes.Contains(a->GetTypeHandle())) { - ConcurrentSystemLocker::WriteScope lock(Locker, true); + ScopeWriteLock lock(Locker); OnSceneRenderingDirty(a->GetBox()); } } @@ -407,7 +407,7 @@ public: { if (GLOBAL_SDF_ACTOR_IS_STATIC(a) && ObjectTypes.Contains(a->GetTypeHandle())) { - ConcurrentSystemLocker::WriteScope lock(Locker, true); + ScopeWriteLock lock(Locker); OnSceneRenderingDirty(BoundingBox::FromSphere(prevBounds)); OnSceneRenderingDirty(a->GetBox()); } @@ -417,14 +417,14 @@ public: { if (GLOBAL_SDF_ACTOR_IS_STATIC(a) && ObjectTypes.Contains(a->GetTypeHandle())) { - ConcurrentSystemLocker::WriteScope lock(Locker, true); + ScopeWriteLock lock(Locker); OnSceneRenderingDirty(a->GetBox()); } } void OnSceneRenderingClear(SceneRendering* scene) override { - ConcurrentSystemLocker::WriteScope lock(Locker, true); + ScopeWriteLock lock(Locker); for (auto& cascade : Cascades) cascade.StaticChunks.Clear(); } @@ -583,7 +583,7 @@ void GlobalSignDistanceFieldCustomBuffer::DrawCascadeJob(int32 cascadeIndex) if (!cascade.Dirty) return; PROFILE_CPU(); - ConcurrentSystemLocker::ReadScope lock(Locker); + ScopeReadLock lock(Locker); CurrentCascade.Set(&cascade); DrawCascadeActors(cascade); UpdateCascadeChunks(cascade); @@ -798,7 +798,7 @@ bool GlobalSignDistanceFieldPass::Render(RenderContext& renderContext, GPUContex Current = &sdfData; sdfData.StartDrawing(renderContext, false, reset); // (ignored if not started earlier this frame) sdfData.WaitForDrawing(); - ConcurrentSystemLocker::WriteScope lock(sdfData.Locker); + ScopeWriteLock lock(sdfData.Locker); // Rasterize world geometry into Global SDF bool anyDraw = false; diff --git a/Source/Engine/Threading/ConcurrentSystemLocker.cpp b/Source/Engine/Threading/ConcurrentSystemLocker.cpp deleted file mode 100644 index 4b64a12ac..000000000 --- a/Source/Engine/Threading/ConcurrentSystemLocker.cpp +++ /dev/null @@ -1,75 +0,0 @@ -// Copyright (c) Wojciech Figat. All rights reserved. - -#include "ConcurrentSystemLocker.h" -#include "Engine/Platform/Platform.h" -#if !BUILD_RELEASE -#include "Engine/Core/Log.h" -#endif - -ConcurrentSystemLocker::ConcurrentSystemLocker() -{ - _counters[0] = _counters[1] = 0; -} - -void ConcurrentSystemLocker::Begin(bool write, bool exclusively) -{ - volatile int64* thisCounter = &_counters[write]; - volatile int64* otherCounter = &_counters[!write]; - -#if !BUILD_RELEASE - int32 retries = 0; - double startTime = Platform::GetTimeSeconds(); -#endif -RETRY: -#if !BUILD_RELEASE - retries++; - if (retries > 1000) - { - double endTime = Platform::GetTimeSeconds(); - if (endTime - startTime > 0.5f) - { - LOG(Error, "Deadlock detected in ConcurrentSystemLocker! Thread 0x{0:x} waits for {1} ms...", Platform::GetCurrentThreadID(), (int32)((endTime - startTime) * 1000.0)); - retries = 0; - } - } -#endif - - // Check if we can enter (cannot read while someone else is writing and vice versa) - if (Platform::AtomicRead(otherCounter) != 0) - { - // Someone else is doing opposite operation so wait for it's end - // TODO: use ConditionVariable+CriticalSection to prevent active-waiting - Platform::Yield(); - goto RETRY; - } - - // Writers might want to check themselves for a single writer at the same time - just like a mutex - if (exclusively && Platform::AtomicRead(thisCounter) != 0) - { - // Someone else is doing opposite operation so wait for it's end - Platform::Yield(); - goto RETRY; - } - - // Mark that we entered this section - Platform::InterlockedIncrement(thisCounter); - - // Double-check if we're safe to go - if (Platform::InterlockedCompareExchange(otherCounter, 0, 0)) - { - // Someone else is doing opposite operation while this thread was doing counter increment so retry - Platform::InterlockedDecrement(thisCounter); - goto RETRY; - } -} - -void ConcurrentSystemLocker::End(bool write) -{ - // Mark that we left this section - Platform::InterlockedDecrement(&_counters[write]); -} - -bool ConcurrentSystemLocker::HasLock(bool write) const -{ - return Platform::AtomicRead(&_counters[write]) != 0; -} diff --git a/Source/Engine/Threading/ConcurrentSystemLocker.h b/Source/Engine/Threading/ConcurrentSystemLocker.h deleted file mode 100644 index 0b46a64f5..000000000 --- a/Source/Engine/Threading/ConcurrentSystemLocker.h +++ /dev/null @@ -1,47 +0,0 @@ -// Copyright (c) Wojciech Figat. All rights reserved. - -#pragma once - -#include "Engine/Core/Core.h" -#include "Engine/Core/Types/BaseTypes.h" - -/// -/// Utility for guarding system data access from different threads depending on the resources usage (eg. block read on write). -/// -struct ConcurrentSystemLocker -{ -private: - volatile int64 _counters[2]; - -public: - NON_COPYABLE(ConcurrentSystemLocker); - ConcurrentSystemLocker(); - - void Begin(bool write, bool exclusively = false); - void End(bool write); - bool HasLock(bool write) const; - -public: - template - struct Scope - { - NON_COPYABLE(Scope); - - Scope(ConcurrentSystemLocker& locker, bool exclusively = false) - : _locker(locker) - { - _locker.Begin(Write, exclusively); - } - - ~Scope() - { - _locker.End(Write); - } - - private: - ConcurrentSystemLocker& _locker; - }; - - typedef Scope ReadScope; - typedef Scope WriteScope; -}; diff --git a/Source/Engine/Threading/Threading.h b/Source/Engine/Threading/Threading.h index 101d058d0..a075351ea 100644 --- a/Source/Engine/Threading/Threading.h +++ b/Source/Engine/Threading/Threading.h @@ -3,6 +3,7 @@ #pragma once #include "Engine/Platform/CriticalSection.h" +#include "Engine/Platform/ReadWriteLock.h" /// /// Checks if current execution in on the main thread. @@ -10,35 +11,70 @@ FLAXENGINE_API bool IsInMainThread(); /// -/// Scope locker for critical section. +/// Scope lock for critical section (mutex). Ensures no other thread can enter scope. /// class ScopeLock { private: - const CriticalSection* _section; - - ScopeLock() = default; - ScopeLock(const ScopeLock&) = delete; - ScopeLock& operator=(const ScopeLock&) = delete; + ScopeLock() = delete; + NON_COPYABLE(ScopeLock); public: - - /// - /// Init, enters critical section. - /// - /// The synchronization object to lock. - ScopeLock(const CriticalSection& section) + FORCE_INLINE ScopeLock(const CriticalSection& section) : _section(§ion) { _section->Lock(); } - /// - /// Destructor, releases critical section. - /// - ~ScopeLock() + FORCE_INLINE ~ScopeLock() { _section->Unlock(); } }; + +/// +/// Scope lock for read/write lock that allows for shared reading by multiple threads (no writers allowed). +/// +class ScopeReadLock +{ +private: + const ReadWriteLock* _lock; + ScopeReadLock() = delete; + NON_COPYABLE(ScopeReadLock); + +public: + FORCE_INLINE ScopeReadLock(const ReadWriteLock& lock) + : _lock(&lock) + { + _lock->ReadLock(); + } + + FORCE_INLINE ~ScopeReadLock() + { + _lock->ReadUnlock(); + } +}; + +/// +/// Scope lock for read/write lock that allows for exclusive writing by a single thread (no readers allowed). +/// +class ScopeWriteLock +{ +private: + const ReadWriteLock* _lock; + ScopeWriteLock() = delete; + NON_COPYABLE(ScopeWriteLock); + +public: + FORCE_INLINE ScopeWriteLock(const ReadWriteLock& lock) + : _lock(&lock) + { + _lock->WriteLock(); + } + + FORCE_INLINE ~ScopeWriteLock() + { + _lock->WriteUnlock(); + } +};