From b8c9130ae42e9dc5358f3e14e8e19fd2eeb9b600 Mon Sep 17 00:00:00 2001 From: Wojtek Figat Date: Tue, 11 Jun 2024 12:53:47 +0200 Subject: [PATCH] Simplify and cleanup code #2368 --- Source/Engine/Core/LogContext.cpp | 75 ++++++++++++++------- Source/Engine/Core/LogContext.h | 67 +++++++----------- Source/Engine/Level/SceneObjectsFactory.cpp | 2 + Source/Engine/Scripting/Script.cpp | 6 -- Source/Engine/Scripting/Scripting.cpp | 6 +- Source/Engine/Scripting/ScriptingObject.cpp | 6 +- 6 files changed, 84 insertions(+), 78 deletions(-) diff --git a/Source/Engine/Core/LogContext.cpp b/Source/Engine/Core/LogContext.cpp index 9b649885b..07d0859a7 100644 --- a/Source/Engine/Core/LogContext.cpp +++ b/Source/Engine/Core/LogContext.cpp @@ -1,42 +1,67 @@ -#include "LogContexts.h" +// Copyright (c) 2012-2024 Wojciech Figat. All rights reserved. + +#include "LogContext.h" #include "Engine/Core/Types/Guid.h" #include "Engine/Core/Types/String.h" +#include "Engine/Core/Collections/Array.h" +#include "Engine/Threading/ThreadLocal.h" -ThreadLocal LogContexts::CurrentLogContext; - -String LogContextFormatter::Format() +struct LogContextThreadData { - LogContextData context = LogContexts::Get(); + LogContextData* Ptr; + uint32 Count; + uint32 Capacity; + + void Push(LogContextData&& item) + { + if (Count == Capacity) + { + Capacity = Capacity == 0 ? 32 : Capacity * 2; + auto ptr = (LogContextData*)Allocator::Allocate(Capacity * sizeof(LogContextData)); + Platform::MemoryCopy(ptr, Ptr, Count * sizeof(LogContextData)); + Allocator::Free(Ptr); + Ptr = ptr; + } + Ptr[Count++] = MoveTemp(item); + } + + void Pop() + { + Count--; + } + + LogContextData Peek() + { + return Count == 0 ? Ptr[Count] : LogContextData(); + } +}; + +ThreadLocal GlobalLogContexts; + +String LogContext::GetInfo() +{ + LogContextData context = LogContext::Get(); if (context.ObjectID != Guid::Empty) return String::Format(TEXT("(Loading source was {0})"), context.ObjectID); - - return String(""); + return String::Empty; } -void LogContexts::Set(const Guid& id) +void LogContext::Push(const Guid& id) { - LogContextData context = LogContextData(); + LogContextData context; context.ObjectID = id; - - LogContextStack contextStack = LogContexts::CurrentLogContext.Get(); - contextStack.Stack.Push(context); - - LogContexts::CurrentLogContext.Set(contextStack); + auto& stack = GlobalLogContexts.Get(); + stack.Push(MoveTemp(context)); } -void LogContexts::Clear() +void LogContext::Pop() { - LogContextStack contextStack = LogContexts::CurrentLogContext.Get(); - contextStack.Stack.Pop(); - - LogContexts::CurrentLogContext.Set(contextStack); + auto& stack = GlobalLogContexts.Get(); + stack.Pop(); } -LogContextData LogContexts::Get() +LogContextData LogContext::Get() { - LogContextStack contextStack = LogContexts::CurrentLogContext.Get(); - if (contextStack.Stack.Count() == 0) - return LogContextData(); - - return contextStack.Stack.Last(); + auto& stack = GlobalLogContexts.Get(); + return stack.Peek(); } diff --git a/Source/Engine/Core/LogContext.h b/Source/Engine/Core/LogContext.h index 7e15e40f7..8653d0404 100644 --- a/Source/Engine/Core/LogContext.h +++ b/Source/Engine/Core/LogContext.h @@ -1,8 +1,9 @@ +// Copyright (c) 2012-2024 Wojciech Figat. All rights reserved. + #pragma once + #include "Engine/Core/Config.h" #include "Engine/Scripting/ScriptingType.h" -#include "Engine/Threading/ThreadLocal.h" -#include "Engine/Core/Collections/Array.h" class String; struct Guid; @@ -10,7 +11,7 @@ struct Guid; /// /// Log context data structure. Contains different kinds of context data for different situtations. /// -API_STRUCT() struct FLAXENGINE_API LogContextData +API_STRUCT(NoDefault) struct FLAXENGINE_API LogContextData { DECLARE_SCRIPTING_TYPE_STRUCTURE(LogContextData) @@ -26,44 +27,25 @@ struct TIsPODType enum { Value = true }; }; -/// -/// Stores a stack of log contexts. -/// -API_STRUCT() struct FLAXENGINE_API LogContextStack -{ - DECLARE_SCRIPTING_TYPE_STRUCTURE(LogContextStack) - - /// - /// Log context stack. - /// - Array Stack; -}; - -template<> -struct TIsPODType -{ - enum { Value = true }; -}; - /// /// Log context interaction class. Methods are thread local, and as such, the context is as well. /// This system is used to pass down important information to be logged through large callstacks /// which don't have any reason to be passing down the information otherwise. /// -API_CLASS(Static) class FLAXENGINE_API LogContexts +API_CLASS(Static) class FLAXENGINE_API LogContext { - DECLARE_SCRIPTING_TYPE_MINIMAL(LogContexts) + DECLARE_SCRIPTING_TYPE_MINIMAL(LogContext) /// /// Adds a log context element to the stack to be displayed in warning and error logs. /// - /// The GUID of the object this context applies to. - API_FUNCTION() static void Set(const Guid& id); + /// The ID of the object this context applies to. + API_FUNCTION() static void Push(const Guid& id); /// /// Pops a log context element off of the stack and discards it. /// - API_FUNCTION() static void Clear(); + API_FUNCTION() static void Pop(); /// /// Gets the log context element off the top of stack. @@ -71,22 +53,25 @@ API_CLASS(Static) class FLAXENGINE_API LogContexts /// The log context element at the top of the stack. API_FUNCTION() static LogContextData Get(); -private: - static ThreadLocal CurrentLogContext; -}; - -/// -/// Formatting class which will provide a string to apply -/// the current log context to an error or warning. -/// -API_CLASS(Static) class FLAXENGINE_API LogContextFormatter -{ - DECLARE_SCRIPTING_TYPE_MINIMAL(LogContextFormatter) - -public: /// /// Returns a string which represents the current log context on the stack. /// /// The formatted string representing the current log context. - API_FUNCTION() static String Format(); + API_FUNCTION() static String GetInfo(); +}; + +/// +/// Helper structure used to call LogContext Push/Pop within single code block. +/// +struct LogContextScope +{ + FORCE_INLINE LogContextScope(const Guid& id) + { + LogContext::Push(id); + } + + FORCE_INLINE ~LogContextScope() + { + LogContext::Pop(); + } }; diff --git a/Source/Engine/Level/SceneObjectsFactory.cpp b/Source/Engine/Level/SceneObjectsFactory.cpp index 3df67d8ba..dd79f1481 100644 --- a/Source/Engine/Level/SceneObjectsFactory.cpp +++ b/Source/Engine/Level/SceneObjectsFactory.cpp @@ -6,6 +6,7 @@ #include "Engine/Content/Content.h" #include "Engine/Core/Cache.h" #include "Engine/Core/Log.h" +#include "Engine/Core/LogContext.h" #include "Engine/Scripting/Scripting.h" #include "Engine/Serialization/JsonTools.h" #include "Engine/Serialization/ISerializeModifier.h" @@ -247,6 +248,7 @@ void SceneObjectsFactory::Deserialize(Context& context, SceneObject* obj, ISeria CHECK(obj); #endif ISerializeModifier* modifier = context.GetModifier(); + LogContextScope logContext(obj->GetID()); // Check for prefab instance Guid prefabObjectId; diff --git a/Source/Engine/Scripting/Script.cpp b/Source/Engine/Scripting/Script.cpp index e6314eb1d..5bee98436 100644 --- a/Source/Engine/Scripting/Script.cpp +++ b/Source/Engine/Scripting/Script.cpp @@ -2,7 +2,6 @@ #include "Script.h" #include "Engine/Core/Log.h" -#include "Engine/Core/Types/LogContexts.h" #if USE_EDITOR #include "Internal/StdTypesContainer.h" #include "ManagedCLR/MClass.h" @@ -331,9 +330,6 @@ void Script::Serialize(SerializeStream& stream, const void* otherObj) void Script::Deserialize(DeserializeStream& stream, ISerializeModifier* modifier) { - // Store Logging Context - LogContexts::Set(GetID()); - // Base SceneObject::Deserialize(stream, modifier); @@ -368,6 +364,4 @@ void Script::Deserialize(DeserializeStream& stream, ISerializeModifier* modifier } } } - - LogContexts::Clear(); } diff --git a/Source/Engine/Scripting/Scripting.cpp b/Source/Engine/Scripting/Scripting.cpp index 8c7405963..67806a2ee 100644 --- a/Source/Engine/Scripting/Scripting.cpp +++ b/Source/Engine/Scripting/Scripting.cpp @@ -21,10 +21,10 @@ #include "ManagedCLR/MCore.h" #include "ManagedCLR/MException.h" #include "Internal/StdTypesContainer.h" +#include "Engine/Core/LogContext.h" #include "Engine/Core/ObjectsRemovalService.h" #include "Engine/Core/Types/TimeSpan.h" #include "Engine/Core/Types/Stopwatch.h" -#include "Engine/Core/Types/LogContexts.h" #include "Engine/Content/Asset.h" #include "Engine/Content/Content.h" #include "Engine/Engine/EngineService.h" @@ -881,7 +881,7 @@ ScriptingObject* Scripting::FindObject(Guid id, const MClass* type) // Check type if (!type || result->Is(type)) return result; - LOG(Warning, "Found scripting object with ID={0} of type {1} that doesn't match type {2}. {3}", id, String(result->GetType().Fullname), String(type->GetFullName()), LogContextFormatter::Format()); + LOG(Warning, "Found scripting object with ID={0} of type {1} that doesn't match type {2}. {3}", id, String(result->GetType().Fullname), String(type->GetFullName()), LogContext::GetInfo()); return nullptr; } @@ -900,7 +900,7 @@ ScriptingObject* Scripting::FindObject(Guid id, const MClass* type) return asset; } - LOG(Warning, "Unable to find scripting object with ID={0}. Required type {1}. {2}", id, String(type->GetFullName()), LogContextFormatter::Format()); + LOG(Warning, "Unable to find scripting object with ID={0}. Required type {1}. {2}", id, String(type->GetFullName()), LogContext::GetInfo()); return nullptr; } diff --git a/Source/Engine/Scripting/ScriptingObject.cpp b/Source/Engine/Scripting/ScriptingObject.cpp index b5dff92bc..3e88df30c 100644 --- a/Source/Engine/Scripting/ScriptingObject.cpp +++ b/Source/Engine/Scripting/ScriptingObject.cpp @@ -6,8 +6,8 @@ #include "BinaryModule.h" #include "Engine/Level/Actor.h" #include "Engine/Core/Log.h" +#include "Engine/Core/LogContext.h" #include "Engine/Core/Types/Pair.h" -#include "Engine/Core/Types/LogContexts.h" #include "Engine/Utilities/StringConverter.h" #include "Engine/Content/Asset.h" #include "Engine/Content/Content.h" @@ -727,7 +727,7 @@ DEFINE_INTERNAL_CALL(MObject*) ObjectInternal_FindObject(Guid* id, MTypeObject* if (klass && !obj->Is(klass)) { if (!skipLog) - LOG(Warning, "Found scripting object with ID={0} of type {1} that doesn't match type {2}. {3}", *id, String(obj->GetType().Fullname), String(klass->GetFullName()), LogContextFormatter::Format()); + LOG(Warning, "Found scripting object with ID={0} of type {1} that doesn't match type {2}. {3}", *id, String(obj->GetType().Fullname), String(klass->GetFullName()), LogContext::GetInfo()); return nullptr; } return obj->GetOrCreateManagedInstance(); @@ -736,7 +736,7 @@ DEFINE_INTERNAL_CALL(MObject*) ObjectInternal_FindObject(Guid* id, MTypeObject* if (!skipLog) { if (klass) - LOG(Warning, "Unable to find scripting object with ID={0}. Required type {1}. {2}", *id, String(klass->GetFullName()), LogContextFormatter::Format()); + LOG(Warning, "Unable to find scripting object with ID={0}. Required type {1}. {2}", *id, String(klass->GetFullName()), LogContext::GetInfo()); else LOG(Warning, "Unable to find scripting object with ID={0}", *id); }