From 933432e62d68169d32929736c4acab1b33f0ce05 Mon Sep 17 00:00:00 2001 From: sfan5 Date: Thu, 15 Feb 2024 16:24:15 +0100 Subject: [PATCH] Annotate Lua packer with more comments --- src/script/common/c_packer.cpp | 93 ++++++++++++++++++++++++++++------ src/script/common/c_packer.h | 15 +++--- 2 files changed, 85 insertions(+), 23 deletions(-) diff --git a/src/script/common/c_packer.cpp b/src/script/common/c_packer.cpp index 597f5e447..36b3f4dcc 100644 --- a/src/script/common/c_packer.cpp +++ b/src/script/common/c_packer.cpp @@ -70,6 +70,7 @@ static inline bool uses_union(int type) } } +// can set_into be used with these key / value types in principle? static inline bool can_set_into(int ktype, int vtype) { switch (ktype) { @@ -82,7 +83,7 @@ static inline bool can_set_into(int ktype, int vtype) } } -// is the key suitable for use with set_into? +// is the actual key suitable for use with set_into? static inline bool suitable_key(lua_State *L, int idx) { if (lua_type(L, idx) == LUA_TSTRING) { @@ -143,6 +144,13 @@ namespace { typedef std::pair PackerTuple; } +/** + * Append instruction to end. + * + * @param pv target + * @param type instruction type + * @return reference to instruction +*/ static inline auto emplace(PackedValue &pv, s16 type) { pv.i.emplace_back(); @@ -211,6 +219,13 @@ void script_register_packer(lua_State *L, const char *regname, lua_pop(L, 1); } +/** + * Find a packer for a metatable. + * + * @param regname metatable name + * @param out packer will be placed here + * @return success +*/ static bool find_packer(const char *regname, PackerTuple &out) { MutexAutoLock autolock(g_packers_lock); @@ -223,6 +238,14 @@ static bool find_packer(const char *regname, PackerTuple &out) return true; } +/** + * Find a packer matching the metatable of the Lua value. + * + * @param L Lua state + * @param idx Index on stack + * @param out packer will be placed here + * @return success +*/ static bool find_packer(lua_State *L, int idx, PackerTuple &out) { #ifndef NDEBUG @@ -254,6 +277,21 @@ static bool find_packer(lua_State *L, int idx, PackerTuple &out) // Packing implementation // +/** + * Keeps track of seen objects, which is needed to make circular references work. + * The first time an object is seen it remembers the instruction index. + * The caller is expected to add instructions that produce the value immediately after. + * For second, third, ... calls it pushes an instruction that references the already + * created value. + * + * @param L Lua state + * @param idx Index of value on Lua stack + * @param pv target + * @param seen Map of seen objects + * @return empty reference (first time) or reference to instruction that + * reproduces the value (otherwise) + * +*/ static VectorRef record_object(lua_State *L, int idx, PackedValue &pv, std::unordered_map &seen) { @@ -261,18 +299,31 @@ static VectorRef record_object(lua_State *L, int idx, PackedValue & assert(ptr); auto found = seen.find(ptr); if (found == seen.end()) { + // first time, record index + assert(pv.i.size() <= S32_MAX); seen[ptr] = pv.i.size(); return VectorRef(); } + s32 ref = found->second; assert(ref < (s32)pv.i.size()); // reuse the value from first time auto r = emplace(pv, INSTR_PUSHREF); - r->ref = ref; + r->sidata1 = ref; pv.i[ref].keep_ref = true; return r; } +/** + * Pack a single Lua value and add it to the instruction stream. + * + * @param L Lua state + * @param idx Index of value on Lua stack. Must be positive, use absidx if not! + * @param vidx Next free index on the stack as it would look during unpacking. (v = virtual) + * @param pv target + * @param seen Map of seen objects (see record_object) + * @return reference to the instruction that creates the value +*/ static VectorRef pack_inner(lua_State *L, int idx, int vidx, PackedValue &pv, std::unordered_map &seen) { @@ -330,6 +381,7 @@ static VectorRef pack_inner(lua_State *L, int idx, int vidx, Packed PackerTuple ser; if (!find_packer(L, idx, ser)) throw LuaError("Cannot serialize unsupported userdata"); + // use packer callback to turn into a void* pv.contains_userdata = true; r = emplace(pv, LUA_TUSERDATA); r->sdata = ser.first; @@ -358,22 +410,28 @@ static VectorRef pack_inner(lua_State *L, int idx, int vidx, Packed else rtable->uidata2++; // nrec - // check if we can use a shortcut + // set_into is a shortcut that allows a pushed value + // to be directly set into a table without separately pushing + // the key and using SETTABLE. + // only works in certain circumstances, hence the check: if (can_set_into(ktype, vtype) && suitable_key(L, -2)) { // push only the value auto rval = pack_inner(L, absidx(L, -1), vidx, pv, seen); + vidx++; rval->pop = rval->type != LUA_TTABLE; - // and where to put it: + // where to put it: rval->set_into = vi_table; if (ktype == LUA_TSTRING) rval->sdata = lua_tostring(L, -2); else rval->sidata1 = lua_tointeger(L, -2); - // pop tables after the fact + // since tables take multiple instructions to populate we have to + // pop them separately afterwards. if (!rval->pop) { auto ri1 = emplace(pv, INSTR_POP); - ri1->sidata1 = vidx; + ri1->sidata1 = vidx - 1; } + vidx--; } else { // push the key and value pack_inner(L, absidx(L, -2), vidx, pv, seen); @@ -392,6 +450,7 @@ static VectorRef pack_inner(lua_State *L, int idx, int vidx, Packed lua_pop(L, 1); } + // exactly the table should be left on stack assert(vidx == vi_table + 1); return rtable; } @@ -405,6 +464,7 @@ PackedValue *script_pack(lua_State *L, int idx) std::unordered_map seen; pack_inner(L, idx, 1, pv, seen); + // allocate last for exception safety return new PackedValue(std::move(pv)); } @@ -414,14 +474,15 @@ PackedValue *script_pack(lua_State *L, int idx) void script_unpack(lua_State *L, PackedValue *pv) { - lua_newtable(L); // table at index top to track ref indices -> objects + // table that tracks objects for keep_ref / PUSHREF (key = instr index) + lua_newtable(L); const int top = lua_gettop(L); int ctr = 0; for (size_t packed_idx = 0; packed_idx < pv->i.size(); packed_idx++) { auto &i = pv->i[packed_idx]; - // If leaving values on stack make sure there's space (every 5th iteration) + // Make sure there's space on the stack (if applicable) if (!i.pop && (ctr++) >= 5) { lua_checkstack(L, 5); ctr = 0; @@ -449,7 +510,8 @@ void script_unpack(lua_State *L, PackedValue *pv) lua_remove(L, top + i.sidata2); continue; case INSTR_PUSHREF: - lua_pushinteger(L, i.ref); + // retrieve from top table + lua_pushinteger(L, i.sidata1); lua_rawget(L, top); break; @@ -476,7 +538,7 @@ void script_unpack(lua_State *L, PackedValue *pv) PackerTuple ser; sanity_check(find_packer(i.sdata.c_str(), ser)); ser.second.fout(L, i.ptrdata); - i.ptrdata = nullptr; // ownership taken by callback + i.ptrdata = nullptr; // ownership taken by packer callback break; } @@ -486,13 +548,14 @@ void script_unpack(lua_State *L, PackedValue *pv) } if (i.keep_ref) { + // remember in top table lua_pushinteger(L, packed_idx); lua_pushvalue(L, -2); lua_rawset(L, top); } if (i.set_into) { - if (!i.pop) + if (!i.pop) // set will consume lua_pushvalue(L, -1); if (uses_sdata(i.type)) lua_rawseti(L, top + i.set_into, i.sidata1); @@ -504,7 +567,7 @@ void script_unpack(lua_State *L, PackedValue *pv) } } - // as part of the unpacking process we take ownership of all userdata + // as part of the unpacking process all userdata is "used up" pv->contains_userdata = false; // leave exactly one value on the stack lua_settop(L, top+1); @@ -523,7 +586,7 @@ PackedValue::~PackedValue() if (i.type == LUA_TUSERDATA && i.ptrdata) { PackerTuple ser; if (find_packer(i.sdata.c_str(), ser)) { - // tell it to deallocate object + // tell packer to deallocate object ser.second.fout(nullptr, i.ptrdata); } else { assert(false); @@ -536,7 +599,6 @@ PackedValue::~PackedValue() // script_dump_packed // -#ifndef NDEBUG void script_dump_packed(const PackedValue *val) { printf("instruction stream: [\n"); @@ -550,7 +612,7 @@ void script_dump_packed(const PackedValue *val) printf(i.sidata2 ? "POP(%d, %d)" : "POP(%d)", i.sidata1, i.sidata2); break; case INSTR_PUSHREF: - printf("PUSHREF(%d)", i.ref); + printf("PUSHREF(%d)", i.sidata1); break; case LUA_TNIL: printf("nil"); @@ -593,4 +655,3 @@ void script_dump_packed(const PackedValue *val) } printf("]\n"); } -#endif diff --git a/src/script/common/c_packer.h b/src/script/common/c_packer.h index fe072c10a..17f25fd17 100644 --- a/src/script/common/c_packer.h +++ b/src/script/common/c_packer.h @@ -39,30 +39,31 @@ extern "C" { #define INSTR_PUSHREF (-12) /** - * Represents a single instruction that pushes a new value or works with existing ones. + * Represents a single instruction that pushes a new value or operates with existing ones. */ struct PackedInstr { s16 type; // LUA_T* or INSTR_* u16 set_into; // set into table on stack - bool keep_ref; // is referenced later by INSTR_PUSHREF? + bool keep_ref; // referenced later by INSTR_PUSHREF? bool pop; // remove from stack? + // Note: the remaining members are named by type, not usage union { bool bdata; // boolean: value lua_Number ndata; // number: value struct { - u16 uidata1, uidata2; // table: narr, nrec + u16 uidata1, uidata2; // table: narr | nrec }; struct { /* - SETTABLE: key index, value index + SETTABLE: key index | value index POP: indices to remove - otherwise w/ set_into: numeric key, - + PUSHREF: index of referenced instr | unused + otherwise w/ set_into: numeric key | unused */ s32 sidata1, sidata2; }; void *ptrdata; // userdata: implementation defined - s32 ref; // PUSHREF: index of referenced instr }; /* - string: value @@ -78,7 +79,7 @@ struct PackedInstr /** * A packed value can be a primitive like a string or number but also a table * including all of its contents. It is made up of a linear stream of - * 'instructions' that build the final value when executed. + * instructions that build the final value when executed. */ struct PackedValue {