From 0cc7436be1f57299384c5acad5d32e13f2f4d1cf Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Thu, 27 Jun 2024 19:47:00 -0400 Subject: [PATCH] Introduce TypeTag template whose int value differs for each T. This replaces type_tag(), which searched and possibly extended the type_tags unordered_map at runtime. If we called lua_emplace() from different threads, that would require locking type_tags. In contrast, the compiler must instantiate a distinct TypeTag for every distinct T passed to lua_emplace(), so each gets a distinct value at static initialization time. No locking is required; no lookup; no allocations. Add a test to llluamanager_test.cpp to verify that each distinct T passed to lua_emplace() gets its own TypeTag::value, and that each gets its own destructor -- but that different lua_emplace() calls with the same T share the same TypeTag::value and the same destructor. --- indra/llcommon/lua_function.cpp | 2 ++ indra/llcommon/lua_function.h | 44 +++++++++++++---------- indra/newview/tests/llluamanager_test.cpp | 35 ++++++++++++++++++ 3 files changed, 63 insertions(+), 18 deletions(-) diff --git a/indra/llcommon/lua_function.cpp b/indra/llcommon/lua_function.cpp index 255385b8c44..2d08de68c5b 100644 --- a/indra/llcommon/lua_function.cpp +++ b/indra/llcommon/lua_function.cpp @@ -38,6 +38,8 @@ const S32 INTERRUPTS_SUSPEND_LIMIT = 100; #define lua_register(L, n, f) (lua_pushcfunction(L, (f), n), lua_setglobal(L, (n))) #define lua_rawlen lua_objlen +int DistinctInt::mValues{0}; + /***************************************************************************** * luau namespace *****************************************************************************/ diff --git a/indra/llcommon/lua_function.h b/indra/llcommon/lua_function.h index 284f911fa88..c32a586d79c 100644 --- a/indra/llcommon/lua_function.h +++ b/indra/llcommon/lua_function.h @@ -195,26 +195,34 @@ int name##_luasub::call(lua_State* L) /***************************************************************************** * lua_emplace(), lua_toclass() *****************************************************************************/ -namespace { +// Every instance of DistinctInt has a different int value, barring int +// wraparound. +class DistinctInt +{ +public: + DistinctInt(): mValue(++mValues) {} + int get() const { return mValue; } + operator int() const { return mValue; } +private: + static int mValues; + int mValue; +}; -// If we start engaging lua_emplace() from more than one thread, type_tags -// will need locking. -std::unordered_map type_tags; +namespace { -// find or create a new Luau userdata "tag" for type T template -int type_tag() +struct TypeTag { - // The first time we encounter a given type T, assign a new distinct tag - // value based on the number of previously-created tags. But avoid tag 0, - // which is evidently the default for userdata objects created without - // explicit tags. Don't try to destroy a nonexistent T object in a random - // userdata object! - auto [entry, created] = type_tags.emplace(std::type_index(typeid(T)), int(type_tags.size()+1)); - // Luau only permits up to LUA_UTAG_LIMIT distinct userdata tags (ca. 128) - llassert(entry->second < LUA_UTAG_LIMIT); - return entry->second; -} + // For (std::is_same), &TypeTag::value == &TypeTag::value. + // For (! std::is_same), &TypeTag::value != &TypeTag::value. + // And every distinct instance of DistinctInt has a distinct value. + // Therefore, TypeTag::value is an int uniquely associated with each + // distinct T. + static DistinctInt value; +}; + +template +DistinctInt TypeTag::value; } // anonymous namespace @@ -233,7 +241,7 @@ template void lua_emplace(lua_State* L, ARGS&&... args) { luaL_checkstack(L, 1, nullptr); - int tag{ type_tag() }; + int tag{ TypeTag::value }; if (! lua_getuserdatadtor(L, tag)) { // We haven't yet told THIS lua_State the destructor to use for this tag. @@ -267,7 +275,7 @@ template T* lua_toclass(lua_State* L, int index) { // get void* pointer to userdata (if that's what it is) - void* ptr{ lua_touserdatatagged(L, index, type_tag()) }; + void* ptr{ lua_touserdatatagged(L, index, TypeTag::value) }; // Derive the T* from ptr. If in future lua_emplace() must manually // align our T* within the Lua-provided void*, adjust accordingly. return static_cast(ptr); diff --git a/indra/newview/tests/llluamanager_test.cpp b/indra/newview/tests/llluamanager_test.cpp index 2d525f7913c..d3fc70dfd57 100644 --- a/indra/newview/tests/llluamanager_test.cpp +++ b/indra/newview/tests/llluamanager_test.cpp @@ -465,4 +465,39 @@ namespace tut ensure_equals(desc + " count: " + result.asString(), count, -1); ensure_contains(desc + " result", result.asString(), "terminated"); } + + template + struct Visible + { + Visible(T name): name(name) + { + LL_INFOS() << "Visible<" << LLError::Log::classname() << ">('" << name << "')" << LL_ENDL; + } + Visible(const Visible&) = delete; + Visible& operator=(const Visible&) = delete; + ~Visible() + { + LL_INFOS() << "~Visible<" << LLError::Log::classname() << ">('" << name << "')" << LL_ENDL; + } + T name; + }; + + template<> template<> + void object::test<9>() + { + set_test_name("track distinct lua_emplace() types"); + LuaState L; + lua_emplace>(L, "std::string 0"); + int st0tag = lua_userdatatag(L, -1); + lua_emplace>(L, "const char* 0"); + int cp0tag = lua_userdatatag(L, -1); + lua_emplace>(L, "std::string 1"); + int st1tag = lua_userdatatag(L, -1); + lua_emplace>(L, "const char* 1"); + int cp1tag = lua_userdatatag(L, -1); + lua_settop(L, 0); + ensure_equals("lua_emplace() tags diverge", st0tag, st1tag); + ensure_equals("lua_emplace() tags diverge", cp0tag, cp1tag); + ensure_not_equals("lua_emplace<>() tags collide", st0tag, cp0tag); + } } // namespace tut