From ea5a350f83971f19a6d8bcebfe3f63615cc29cb7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lancelot=20de=20Ferri=C3=A8re?= Date: Sun, 19 Jan 2025 15:19:25 +0100 Subject: [PATCH] Cache JS component wrappers --- source/scriptinterface/ScriptInterface.cpp | 2 +- source/scriptinterface/ScriptInterface.h | 4 +- source/simulation2/components/ICmpPlayer.cpp | 2 +- .../scripting/EngineScriptConversions.cpp | 19 ++------- .../simulation2/scripting/ScriptComponent.cpp | 40 ++++++++++--------- .../simulation2/scripting/ScriptComponent.h | 22 +++++----- .../simulation2/system/ComponentManager.cpp | 23 +++++++++++ source/simulation2/system/ComponentManager.h | 11 +++++ source/simulation2/system/IComponent.cpp | 10 ----- source/simulation2/system/IComponent.h | 12 ++++-- source/simulation2/system/Interface.h | 8 +++- source/simulation2/system/InterfaceScripted.h | 18 +++++++++ 12 files changed, 107 insertions(+), 64 deletions(-) diff --git a/source/scriptinterface/ScriptInterface.cpp b/source/scriptinterface/ScriptInterface.cpp index 1d62a8a78b..22deb97e41 100644 --- a/source/scriptinterface/ScriptInterface.cpp +++ b/source/scriptinterface/ScriptInterface.cpp @@ -517,7 +517,7 @@ void ScriptInterface::DefineCustomObjectType(JSClass *clasp, JSNative constructo JSObject* ScriptInterface::CreateCustomObject(const std::string& typeName) const { - std::map::const_iterator it = m_CustomObjectTypes.find(typeName); + std::unordered_map::const_iterator it = m_CustomObjectTypes.find(typeName); if (it == m_CustomObjectTypes.end()) throw PSERROR_Scripting_TypeDoesNotExist(); diff --git a/source/scriptinterface/ScriptInterface.h b/source/scriptinterface/ScriptInterface.h index 4653a11987..9f9af17113 100644 --- a/source/scriptinterface/ScriptInterface.h +++ b/source/scriptinterface/ScriptInterface.h @@ -24,7 +24,7 @@ #include "scriptinterface/ScriptRequest.h" #include "scriptinterface/ScriptTypes.h" -#include +#include ERROR_GROUP(Scripting); ERROR_TYPE(Scripting, SetupFailed); @@ -289,7 +289,7 @@ private: // members have to be called before the custom destructor of ScriptInterface_impl. std::unique_ptr m; - std::map m_CustomObjectTypes; + std::unordered_map m_CustomObjectTypes; }; // Explicitly instantiate void* as that is used for the generic template, diff --git a/source/simulation2/components/ICmpPlayer.cpp b/source/simulation2/components/ICmpPlayer.cpp index 20b56dbc80..1fb07e6eb3 100644 --- a/source/simulation2/components/ICmpPlayer.cpp +++ b/source/simulation2/components/ICmpPlayer.cpp @@ -48,7 +48,7 @@ public: void Deserialize(const CParamNode& paramNode, IDeserializer& deserialize) final { deserialize.Bool("isActive", m_IsActive); - m_Script.Deserialize(paramNode, deserialize, GetEntityId()); + m_Script.Deserialize(GetSimContext().GetComponentManager(), paramNode, deserialize, GetEntityId()); } void HandleMessage(const CMessage& msg, bool global) final diff --git a/source/simulation2/scripting/EngineScriptConversions.cpp b/source/simulation2/scripting/EngineScriptConversions.cpp index 419676da83..7f77629cec 100644 --- a/source/simulation2/scripting/EngineScriptConversions.cpp +++ b/source/simulation2/scripting/EngineScriptConversions.cpp @@ -35,7 +35,7 @@ #define FAIL(msg) STMT(LOGERROR(msg); return false) #define FAIL_VOID(msg) STMT(ScriptException::Raise(rq, msg); return) -template<> void Script::ToJSVal(const ScriptRequest& rq, JS::MutableHandleValue ret, IComponent* const& val) +template<> void Script::ToJSVal(const ScriptRequest& UNUSED(rq), JS::MutableHandleValue ret, IComponent* const& val) { if (val == NULL) { @@ -43,18 +43,8 @@ template<> void Script::ToJSVal(const ScriptRequest& rq, JS::Mutab return; } - // If this is a scripted component, just return the JS object directly - JS::RootedValue instance(rq.cx, val->GetJSInstance()); - if (!instance.isNull()) - { - ret.set(instance); - return; - } - - // Otherwise we need to construct a wrapper object - // (TODO: cache wrapper objects?) - JS::RootedObject obj(rq.cx); - if (!val->NewJSObject(rq.GetScriptInterface(), &obj)) + JS::HandleValue instance(val->GetJSInstance()); + if (instance.isNull()) { // Report as an error, since scripts really shouldn't try to use unscriptable interfaces LOGERROR("IComponent does not have a scriptable interface"); @@ -62,8 +52,7 @@ template<> void Script::ToJSVal(const ScriptRequest& rq, JS::Mutab return; } - JS::SetReservedSlot(obj, ScriptInterface::JSObjectReservedSlots::PRIVATE, JS::PrivateValue(val)); - ret.setObject(*obj); + ret.set(instance); } template<> void Script::ToJSVal(const ScriptRequest& rq, JS::MutableHandleValue ret, CParamNode const& val) diff --git a/source/simulation2/scripting/ScriptComponent.cpp b/source/simulation2/scripting/ScriptComponent.cpp index be4d79143f..b85e0cc19c 100644 --- a/source/simulation2/scripting/ScriptComponent.cpp +++ b/source/simulation2/scripting/ScriptComponent.cpp @@ -25,31 +25,31 @@ #include "scriptinterface/Object.h" #include "simulation2/serialization/ISerializer.h" #include "simulation2/serialization/IDeserializer.h" +#include "simulation2/system/ComponentManager.h" CComponentTypeScript::CComponentTypeScript(const ScriptInterface& scriptInterface, JS::HandleValue instance) : - m_ScriptInterface(scriptInterface) -{ - m_Instance.init(ScriptRequest(m_ScriptInterface).cx, instance); -} + m_ScriptInterface(scriptInterface), m_Instance(instance) +{} -void CComponentTypeScript::Init(const CParamNode& paramNode, entity_id_t ent) +void CComponentTypeScript::Init(CComponentManager& cmpMgr, const CParamNode& paramNode, entity_id_t ent) { + cmpMgr.RegisterTrace(ent, m_Instance); ScriptRequest rq(m_ScriptInterface); - Script::SetProperty(rq, m_Instance, "entity", (int)ent, true, false); - Script::SetProperty(rq, m_Instance, "template", paramNode, true, false); - ScriptFunction::CallVoid(rq, m_Instance, "Init"); + Script::SetProperty(rq, GetInstance(), "entity", (int)ent, true, false); + Script::SetProperty(rq, GetInstance(), "template", paramNode, true, false); + ScriptFunction::CallVoid(rq, GetInstance(), "Init"); } void CComponentTypeScript::Deinit() { ScriptRequest rq(m_ScriptInterface); - ScriptFunction::CallVoid(rq, m_Instance, "Deinit"); + ScriptFunction::CallVoid(rq, GetInstance(), "Deinit"); } bool CComponentTypeScript::HasMessageHandler(const CMessage& msg, const bool global) { const ScriptRequest rq(m_ScriptInterface); - return Script::HasProperty(rq, m_Instance, global ? msg.GetScriptGlobalHandlerName() : + return Script::HasProperty(rq, GetInstance(), global ? msg.GetScriptGlobalHandlerName() : msg.GetScriptHandlerName()); } @@ -61,7 +61,7 @@ void CComponentTypeScript::HandleMessage(const CMessage& msg, bool global) JS::RootedValue msgVal(rq.cx, msg.ToJSValCached(rq)); - if (!ScriptFunction::CallVoid(rq, m_Instance, name, msgVal)) + if (!ScriptFunction::CallVoid(rq, GetInstance(), name, msgVal)) LOGERROR("Script message handler %s failed", name); } @@ -71,26 +71,28 @@ void CComponentTypeScript::Serialize(ISerializer& serialize) try { - serialize.ScriptVal("comp", &m_Instance); + serialize.ScriptVal("comp", GetMutInstance()); } catch(PSERROR_Serialize& err) { int ent = INVALID_ENTITY; - Script::GetProperty(rq, m_Instance, "entity", ent); + Script::GetProperty(rq, GetInstance(), "entity", ent); std::string name = "(error)"; - Script::GetObjectClassName(rq, m_Instance, name); - LOGERROR("Script component %s of entity %i failed to serialize: %s\nSerializing:\n%s", name, ent, err.what(), Script::ToString(rq, &m_Instance)); + Script::GetObjectClassName(rq, GetInstance(), name); + LOGERROR("Script component %s of entity %i failed to serialize: %s\nSerializing:\n%s", name, ent, err.what(), Script::ToString(rq, GetMutInstance())); // Rethrow now that we added more details throw; } } -void CComponentTypeScript::Deserialize(const CParamNode& paramNode, IDeserializer& deserialize, entity_id_t ent) +void CComponentTypeScript::Deserialize(CComponentManager& cmpMgr, const CParamNode& paramNode, IDeserializer& deserialize, entity_id_t ent) { + cmpMgr.RegisterTrace(ent, m_Instance); + ScriptRequest rq(m_ScriptInterface); - Script::SetProperty(rq, m_Instance, "entity", (int)ent, true, false); - Script::SetProperty(rq, m_Instance, "template", paramNode, true, false); + Script::SetProperty(rq, GetInstance(), "entity", (int)ent, true, false); + Script::SetProperty(rq, GetInstance(), "template", paramNode, true, false); - deserialize.ScriptObjectAssign("comp", m_Instance); + deserialize.ScriptObjectAssign("comp", GetInstance()); } diff --git a/source/simulation2/scripting/ScriptComponent.h b/source/simulation2/scripting/ScriptComponent.h index b9cc227eb3..bf47e7e4f7 100644 --- a/source/simulation2/scripting/ScriptComponent.h +++ b/source/simulation2/scripting/ScriptComponent.h @@ -37,22 +37,24 @@ class CComponentTypeScript public: CComponentTypeScript(const ScriptInterface& scriptInterface, JS::HandleValue instance); - JS::Value GetInstance() const { return m_Instance.get(); } + JS::HandleValue GetInstance() const { return JS::HandleValue::fromMarkedLocation(m_Instance.address()); } + JS::MutableHandleValue GetMutInstance() { return JS::MutableHandleValue::fromMarkedLocation(const_cast(m_Instance.address())); } + static void Trace(JSTracer* trc, void* data); - void Init(const CParamNode& paramNode, entity_id_t ent); + void Init(CComponentManager& cmpMgr, const CParamNode& paramNode, entity_id_t ent); void Deinit(); bool HasMessageHandler(const CMessage& msg, const bool global); void HandleMessage(const CMessage& msg, bool global); void Serialize(ISerializer& serialize); - void Deserialize(const CParamNode& paramNode, IDeserializer& deserialize, entity_id_t ent); + void Deserialize(CComponentManager& cmpMgr, const CParamNode& paramNode, IDeserializer& deserialize, entity_id_t ent); template R Call(const char* funcname, const Ts&... params) const { R ret; ScriptRequest rq(m_ScriptInterface); - if (ScriptFunction::Call(rq, m_Instance, funcname, ret, params...)) + if (ScriptFunction::Call(rq, GetInstance(), funcname, ret, params...)) return ret; LOGERROR("Error calling component script function %s", funcname); return R(); @@ -63,7 +65,7 @@ public: void CallRef(const char* funcname, R ret, const Ts&... params) const { ScriptRequest rq(m_ScriptInterface); - if (!ScriptFunction::Call(rq, m_Instance, funcname, ret, params...)) + if (!ScriptFunction::Call(rq, GetInstance(), funcname, ret, params...)) LOGERROR("Error calling component script function %s", funcname); } @@ -71,13 +73,13 @@ public: void CallVoid(const char* funcname, const Ts&... params) const { ScriptRequest rq(m_ScriptInterface); - if (!ScriptFunction::CallVoid(rq, m_Instance, funcname, params...)) + if (!ScriptFunction::CallVoid(rq, GetInstance(), funcname, params...)) LOGERROR("Error calling component script function %s", funcname); } private: const ScriptInterface& m_ScriptInterface; - JS::PersistentRootedValue m_Instance; + JS::Heap m_Instance; }; #define REGISTER_COMPONENT_SCRIPT_WRAPPER(cname) \ @@ -105,13 +107,13 @@ private: } \ void Init(const CParamNode& paramNode) override \ { \ - m_Script.Init(paramNode, GetEntityId()); \ + m_Script.Init(GetSimContext().GetComponentManager(), paramNode, GetEntityId()); \ } \ void Deinit() override \ { \ m_Script.Deinit(); \ } \ - JS::Value GetJSInstance() const override \ + JS::HandleValue GetJSInstance() const override \ { \ return m_Script.GetInstance(); \ } \ @@ -136,7 +138,7 @@ private: } \ void Deserialize(const CParamNode& paramNode, IDeserializer& deserialize) override \ { \ - m_Script.Deserialize(paramNode, deserialize, GetEntityId()); \ + m_Script.Deserialize(GetSimContext().GetComponentManager(), paramNode, deserialize, GetEntityId()); \ } \ DEFAULT_SCRIPT_WRAPPER_BASIC(cname) diff --git a/source/simulation2/system/ComponentManager.cpp b/source/simulation2/system/ComponentManager.cpp index 23ccfc92b6..8b8a44c973 100644 --- a/source/simulation2/system/ComponentManager.cpp +++ b/source/simulation2/system/ComponentManager.cpp @@ -68,6 +68,8 @@ CComponentManager::CComponentManager(CSimContext& context, ScriptContext& cx, bo m_ScriptInterface.SetCallbackData(static_cast (this)); m_ScriptInterface.ReplaceNondeterministicRNG(m_RNG); + JS_AddExtraGCRootsTracer(m_ScriptInterface.GetGeneralJSContext(), Trace, (void*)this); + // For component script tests, the test system sets up its own scripted implementation of // these functions, so we skip registering them here in those cases if (!skipScriptFunctions) @@ -119,6 +121,7 @@ CComponentManager::CComponentManager(CSimContext& context, ScriptContext& cx, bo CComponentManager::~CComponentManager() { + JS_RemoveExtraGCRootsTracer(m_ScriptInterface.GetGeneralJSContext(), Trace, (void*)this); ResetState(); } @@ -494,6 +497,9 @@ void CComponentManager::ResetState() m_DynamicMessageSubscriptionsNonsync.clear(); m_DynamicMessageSubscriptionsNonsyncByComponent.clear(); + // Remove all items we were tracing. + m_TraceCache.clear(); + // Delete all IComponents in reverse order of creation. std::map >::reverse_iterator iit = m_ComponentsByTypeId.rbegin(); for (; iit != m_ComponentsByTypeId.rend(); ++iit) @@ -815,6 +821,19 @@ void CComponentManager::AddMockComponent(CEntityHandle ent, InterfaceId iid, ICo cache->interfaces[iid] = &component; } +void CComponentManager::Trace(JSTracer* trc, void* data) +{ + CComponentManager& cmpMgr = *(static_cast(data)); + for (auto& traceByEnt : cmpMgr.m_TraceCache) + for (JS::Heap* ptr : traceByEnt.second) + JS::TraceEdge(trc, ptr, "ComponentManager::Trace"); +} + +void CComponentManager::RegisterTrace(entity_id_t ent, const JS::Heap& instance) +{ + m_TraceCache.try_emplace(ent).first->second.push_back(const_cast*>(std::addressof(instance))); +} + CEntityHandle CComponentManager::AllocateEntityHandle(entity_id_t ent) { ENSURE(!EntityExists(ent)); @@ -955,6 +974,10 @@ void CComponentManager::FlushDestroyedComponents() free(handle.GetComponentCache()); m_ComponentCaches.erase(ent); + auto hit = m_TraceCache.find(ent); + if (hit != m_TraceCache.end()) + m_TraceCache.erase(hit); + // Remove from m_ComponentsByInterface std::vector >::iterator ifcit = m_ComponentsByInterface.begin(); for (; ifcit != m_ComponentsByInterface.end(); ++ifcit) diff --git a/source/simulation2/system/ComponentManager.h b/source/simulation2/system/ComponentManager.h index f7484b9e27..10769e6149 100644 --- a/source/simulation2/system/ComponentManager.h +++ b/source/simulation2/system/ComponentManager.h @@ -239,6 +239,16 @@ public: */ void FlushDestroyedComponents(); + /** + * Called during GC tracing of our components. + */ + static void Trace(JSTracer* trc, void* data); + /** + * Call this from components when they need to save their JS instance. + * Not done by the component manager because C++ components do it lazily. + */ + void RegisterTrace(entity_id_t ent, const JS::Heap& instance); + IComponent* QueryInterface(entity_id_t ent, InterfaceId iid) const; using InterfacePair = std::pair; @@ -335,6 +345,7 @@ private: std::map > m_DynamicMessageSubscriptionsNonsyncByComponent; std::unordered_map m_ComponentCaches; + std::unordered_map*>> m_TraceCache; // TODO: maintaining both ComponentsBy* is nasty; can we get rid of one, // while keeping QueryInterface and PostMessage sufficiently efficient? diff --git a/source/simulation2/system/IComponent.cpp b/source/simulation2/system/IComponent.cpp index 3c26399ddf..4a526065a8 100644 --- a/source/simulation2/system/IComponent.cpp +++ b/source/simulation2/system/IComponent.cpp @@ -48,13 +48,3 @@ void IComponent::RegisterComponentTypeScriptWrapper(CComponentManager& mgr, EInt void IComponent::HandleMessage(const CMessage& UNUSED(msg), bool UNUSED(global)) { } - -bool IComponent::NewJSObject(const ScriptInterface& UNUSED(scriptInterface), JS::MutableHandleObject UNUSED(out)) const -{ - return false; -} - -JS::Value IComponent::GetJSInstance() const -{ - return JS::NullValue(); -} diff --git a/source/simulation2/system/IComponent.h b/source/simulation2/system/IComponent.h index 9c46c47f02..4a2fa4f590 100644 --- a/source/simulation2/system/IComponent.h +++ b/source/simulation2/system/IComponent.h @@ -66,14 +66,18 @@ public: virtual void Deserialize(const CParamNode& paramNode, IDeserializer& deserialize) = 0; /** - * Returns false by default, indicating that a scripted wrapper of this IComponent is not supported. - * Derrived classes should return true if they implement such a wrapper. + * @Returns JS::NullHandleValue if a scripted wrapper of this IComponent is not supported, the wrapper otherwise. */ - virtual bool NewJSObject(const ScriptInterface& scriptInterface, JS::MutableHandleObject out) const; - virtual JS::Value GetJSInstance() const; + virtual JS::HandleValue GetJSInstance() const = 0; virtual int GetComponentTypeId() const = 0; private: + /** + * @Returns whether a scripted wrapper of this IComponent is not supported. + * Derrived classes should return true if they implement such a wrapper. + */ + virtual bool NewJSObject(const ScriptInterface& scriptInterface, JS::MutableHandleObject out) const = 0; + CEntityHandle m_EntityHandle; const CSimContext* m_SimContext; }; diff --git a/source/simulation2/system/Interface.h b/source/simulation2/system/Interface.h index c0410c248e..e8b17b173f 100644 --- a/source/simulation2/system/Interface.h +++ b/source/simulation2/system/Interface.h @@ -21,8 +21,12 @@ #include "simulation2/system/IComponent.h" #define DECLARE_INTERFACE_TYPE(iname) \ - virtual bool NewJSObject(const ScriptInterface& scriptInterface, JS::MutableHandleObject out) const; \ + virtual JS::HandleValue GetJSInstance() const; \ static void InterfaceInit(ScriptInterface& scriptInterface); \ - static EInterfaceId GetInterfaceId() { return IID_##iname; } + static EInterfaceId GetInterfaceId() { return IID_##iname; } \ +private: \ + virtual bool NewJSObject(const ScriptInterface& scriptInterface, JS::MutableHandleObject out) const; \ + mutable JS::Heap m_CachedInstance; \ +public: #endif // INCLUDED_INTERFACE diff --git a/source/simulation2/system/InterfaceScripted.h b/source/simulation2/system/InterfaceScripted.h index 1473c12a18..8068384b70 100644 --- a/source/simulation2/system/InterfaceScripted.h +++ b/source/simulation2/system/InterfaceScripted.h @@ -19,8 +19,10 @@ #define INCLUDED_INTERFACE_SCRIPTED #include "scriptinterface/FunctionWrapper.h" +#include "scriptinterface/Object.h" #include "scriptinterface/ScriptConversions.h" #include "scriptinterface/ScriptInterface.h" +#include "simulation2/system/ComponentManager.h" #define BEGIN_INTERFACE_WRAPPER(iname) \ JSClass class_ICmp##iname = { \ @@ -37,8 +39,24 @@ bool ICmp##iname::NewJSObject(const ScriptInterface& scriptInterface, JS::MutableHandleObject out) const\ { \ out.set(scriptInterface.CreateCustomObject("ICmp" #iname)); \ + IComponent* comp = const_cast(static_cast(this)); \ + JS::SetReservedSlot(out, ScriptInterface::JSObjectReservedSlots::PRIVATE, JS::PrivateValue(comp)); \ return true; \ } \ + JS::HandleValue ICmp##iname::GetJSInstance() const \ + { \ + if (m_CachedInstance) \ + return JS::HandleValue::fromMarkedLocation(m_CachedInstance.address()); \ + \ + const ScriptInterface& si = GetSimContext().GetScriptInterface(); \ + ScriptRequest rq(si); \ + JS::RootedObject obj(rq.cx); \ + NewJSObject(GetSimContext().GetScriptInterface(), &obj); \ + m_CachedInstance.setObject(*obj); \ + \ + GetSimContext().GetComponentManager().RegisterTrace(GetEntityId(), m_CachedInstance); \ + return JS::HandleValue::fromMarkedLocation(m_CachedInstance.address()); \ + } \ void RegisterComponentInterface_##iname(ScriptInterface& scriptInterface) { \ ICmp##iname::InterfaceInit(scriptInterface); \ }