From 0d5f5f6380a7087f86c5fcd7c761ea5a175c3f17 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Tue, 26 Sep 2023 11:40:56 -0400 Subject: DRTVWR-589: Collect int table keys in vector, not set. Given that we at least have a possibility of determining the length of a Lua table in advance, we might be able to populate a vector of keys with a single initial allocation. Even if Lua reports the length incorrectly, vector::push_back() is one of the bread-and-butter operations of the library, optimized to the extent possible. Inserting elements into a set seems more likely to incur allocations. Of course, we must then sort() the vector to determine its largest key value. Also document the requirement that we use a Lua runtime compiled for C++, that is, compiled to raise errors by C++ exceptions rather than by longjmp(). We rely on temporary stack objects being properly destroyed even if errors are raised. Conventionally, with lua_tomumble(L, index), 'index' refers to the stack index of the Lua object being converted to C++. For a Lua table, talk about table keys rather than table indexes to avoid confusing the maintainer. --- indra/newview/llluamanager.cpp | 70 +++++++++++++++++++++++++++++------------- 1 file changed, 49 insertions(+), 21 deletions(-) diff --git a/indra/newview/llluamanager.cpp b/indra/newview/llluamanager.cpp index fc53117599..f94b26f32e 100644 --- a/indra/newview/llluamanager.cpp +++ b/indra/newview/llluamanager.cpp @@ -53,10 +53,11 @@ extern "C" #include "lua/lualib.h" } +#include #include // std::memcpy() #include -#include #include +#include #if LL_WINDOWS #pragma comment(lib, "liblua54.a") @@ -477,6 +478,17 @@ lua_function(post_on_pump) return 1; } +lua_function(listen_events) +{ + if (! lua_isfunction(L, -1)) + { + return luaL_typerror(L, 1, "function"); + } + // return the distinct LLEventPump name so Lua code can post that with a + // request as the reply pump + return 1; +} + void initLUA(lua_State *L) { LuaFunction::init(L); @@ -571,6 +583,16 @@ std::string lua_tostdstring(lua_State* L, int index) // This function assumes that a Lua caller is fully aware that they're trying // to call a viewer function. In other words, the caller must specifically // construct Lua data convertible to LLSD. +// +// For proper error handling, we REQUIRE that the Lua runtime be compiled as +// C++ so errors are raised as C++ exceptions rather than as longjmp() calls: +// http://www.lua.org/manual/5.4/manual.html#4.4 +// "Internally, Lua uses the C longjmp facility to handle errors. (Lua will +// use exceptions if you compile it as C++; search for LUAI_THROW in the +// source code for details.)" +// Some blocks within this function construct temporary C++ objects in the +// expectation that these objects will be properly destroyed even if code +// reached by that block raises a Lua error. LLSD lua_tollsd(lua_State* L, int index) { switch (lua_type(L, index)) @@ -612,7 +634,7 @@ LLSD lua_tollsd(lua_State* L, int index) { // A Lua table correctly constructed to convert to LLSD will have // either consecutive integer keys starting at 1, which we represent - // as an LLSD array (with Lua index 1 at C++ index 0), or will have + // as an LLSD array (with Lua key 1 at C++ index 0), or will have // all string keys. // // In the belief that Lua table traversal skips "holes," that is, it @@ -652,7 +674,7 @@ LLSD lua_tollsd(lua_State* L, int index) // which can be consumed as either return {}; } - // key is at index -2, value at index -1 + // key is at stack index -2, value at index -1 // from here until lua_next() returns 0, have to lua_pop(2) if we // return early LuaPopper popper(L, 2); @@ -668,11 +690,16 @@ LLSD lua_tollsd(lua_State* L, int index) // same as the length operator '#'; but the length operator states // that it might stop at any "hole" in the subject table. // Moreover, the Lua next() function (and presumably lua_next()) - // traverses a table in unspecified order, even for numeric - // indices (emphasized in the doc). + // traverses a table in unspecified order, even for numeric keys + // (emphasized in the doc). // Make a preliminary pass over the whole table to validate and to - // collect indexes. - std::set indexes; + // collect keys. + std::vector keys; + // Try to determine the length of the table. If the length + // operator is truthful, avoid allocations while we grow the keys + // vector. Even if it's not, we can still grow the vector, albeit + // a little less efficiently. + keys.reserve(luaL_len(L, index)); do { auto arraykeytype{ lua_type(L, -2) }; @@ -680,21 +707,21 @@ LLSD lua_tollsd(lua_State* L, int index) { case LUA_TNUMBER: { - int isnum; - lua_Integer indexint{ lua_tointegerx(L, -2, &isnum) }; - if (! isnum) + int isint; + lua_Integer intkey{ lua_tointegerx(L, -2, &isint) }; + if (! isint) { // key isn't an integer - this doesn't fit our LLSD // array constraints return luaL_error(L, "Expected integer array key, got %f instead", lua_tonumber(L, -2)); } - if (indexint < 1) + if (intkey < 1) { - return luaL_error(L, "array key %d out of bounds", int(indexint)); + return luaL_error(L, "array key %d out of bounds", int(intkey)); } - indexes.insert(LLSD::Integer(indexint)); + keys.push_back(LLSD::Integer(intkey)); break; } @@ -715,28 +742,29 @@ LLSD lua_tollsd(lua_State* L, int index) // Table keys are all integers: are they reasonable integers? // Arbitrary max: may bite us, but more likely to protect us size_t array_max{ 10000 }; - if (indexes.size() > array_max) + if (keys.size() > array_max) { return luaL_error(L, "Conversion from Lua to LLSD array limited to %d entries", int(array_max)); } - // We know the smallest index is >= 1. Check the largest. We also - // know the set is NOT empty, else we wouldn't have gotten here. - LLSD::Integer highindex = *indexes.rbegin(); - if ((highindex - LLSD::Integer(indexes.size())) > 100) + // We know the smallest key is >= 1. Check the largest. We also + // know the vector is NOT empty, else we wouldn't have gotten here. + std::sort(keys.begin(), keys.end()); + LLSD::Integer highkey = *keys.rbegin(); + if ((highkey - LLSD::Integer(keys.size())) > 100) { // Looks like we've gone beyond intentional array gaps into - // crazy index territory. + // crazy key territory. return luaL_error(L, "Gaps in Lua table too large for conversion to LLSD array"); } // right away expand the result array to the size we'll need LLSD result{ LLSD::emptyArray() }; - result[highindex - 1] = LLSD(); + result[highkey - 1] = LLSD(); // Traverse the table again, and this time populate result array. lua_pushnil(L); // first key while (lua_next(L, index)) { - // key at index -2, value at index -1 + // key at stack index -2, value at index -1 // We've already validated lua_tointegerx() for each key. // Don't forget to subtract 1 from Lua key for LLSD subscript! result[LLSD::Integer(lua_tointeger(L, -2)) - 1] = lua_tollsd(L, -1); -- cgit v1.2.3