From c8f6a27e6b27ed5d51cf6c8da5ec080b9952fa99 Mon Sep 17 00:00:00 2001 From: Joshua Haberman Date: Sun, 12 Aug 2018 19:23:26 -0700 Subject: Enforced that upb_msg lives in an Arena only, and other simplifying. upb_msg was trying to be general enough that it could either live in an arena or be allocated with malloc()/free(). This was too much complexity for too little benefit. We should commit to just saying that upb_msg is arena-only. I also ripped out the code to glue upb_msg to the existing handlers-based encoder/decoder. upb_msg has its own, small, simple encoder/decoder. I'm trying to whittle down upb_msg to a small and simple core. I updated the Lua extension for these changes. Lua needs some more work to properly create arenas per message. For now I just created a single global arena. --- upb/bindings/lua/msg.c | 128 ++++++++++++++-------------------------------- upb/bindings/lua/upb.h | 3 +- upb/bindings/lua/upb/pb.c | 117 +++++++----------------------------------- 3 files changed, 58 insertions(+), 190 deletions(-) (limited to 'upb/bindings/lua') diff --git a/upb/bindings/lua/msg.c b/upb/bindings/lua/msg.c index e468ace..e983f46 100644 --- a/upb/bindings/lua/msg.c +++ b/upb/bindings/lua/msg.c @@ -94,56 +94,6 @@ static void *lupb_newuserdata(lua_State *L, size_t size, const char *type) { } -/* lupb_alloc *****************************************************************/ - -typedef struct { - upb_alloc alloc; - lua_State *L; -} lupb_alloc; - -char lupb_alloc_cache_key; - -static void *lupb_alloc_func(upb_alloc *alloc, void *ptr, size_t oldsize, - size_t size) { - lupb_alloc *lalloc = (lupb_alloc*)alloc; - void *ud; - - /* We read this every time in case the user changes the Lua alloc function. */ - lua_Alloc func = lua_getallocf(lalloc->L, &ud); - - return func(ud, ptr, oldsize, size); -} - -static void lupb_alloc_pushnew(lua_State *L) { - lupb_alloc *lalloc = lua_newuserdata(L, sizeof(lupb_alloc)); - - lalloc->alloc.func = &lupb_alloc_func; - lalloc->L = L; -} - -/* Returns the global lupb_alloc func that was created in our luaopen(). - * Callers can be guaranteed that it will be alive as long as |L| is. */ -static upb_alloc *lupb_alloc_get(lua_State *L) { - lupb_alloc *lalloc; - - lua_pushlightuserdata(L, &lupb_alloc_cache_key); - lua_gettable(L, LUA_REGISTRYINDEX); - lalloc = lua_touserdata(L, -1); - UPB_ASSERT(lalloc); - lua_pop(L, 1); - - return &lalloc->alloc; -} - -static void lupb_alloc_initsingleton(lua_State *L) { - lua_pushlightuserdata(L, &lupb_alloc_cache_key); - lupb_alloc_pushnew(L); - lua_settable(L, LUA_REGISTRYINDEX); -} - -#define ADD_BYTES(ptr, bytes) ((void*)((char*)ptr + bytes)) - - /* lupb_arena *****************************************************************/ /* lupb_arena only exists to wrap a upb_arena. It is never exposed to users; @@ -164,6 +114,30 @@ int lupb_arena_new(lua_State *L) { return 1; } +char lupb_arena_cache_key; + +/* Returns the global lupb_arena func that was created in our luaopen(). + * Callers can be guaranteed that it will be alive as long as |L| is. + * TODO(haberman): we shouldn't use a global arena! We should have + * one arena for a parse, or per independently-created message. */ +static upb_arena *lupb_arena_get(lua_State *L) { + upb_arena *arena; + + lua_pushlightuserdata(L, &lupb_arena_cache_key); + lua_gettable(L, LUA_REGISTRYINDEX); + arena = lua_touserdata(L, -1); + UPB_ASSERT(arena); + lua_pop(L, 1); + + return arena; +} + +static void lupb_arena_initsingleton(lua_State *L) { + lua_pushlightuserdata(L, &lupb_arena_cache_key); + lupb_arena_new(L); + lua_settable(L, LUA_REGISTRYINDEX); +} + static int lupb_arena_gc(lua_State *L) { upb_arena *a = lupb_arena_check(L, 1); upb_arena_uninit(a); @@ -314,12 +288,6 @@ const upb_msglayout *lupb_msgclass_getlayout(lua_State *L, int narg) { return lupb_msgclass_check(L, narg)->layout; } -const upb_handlers *lupb_msgclass_getmergehandlers(lua_State *L, int narg) { - const lupb_msgclass *lmsgclass = lupb_msgclass_check(L, narg); - return upb_msgfactory_getmergehandlers( - lmsgclass->lfactory->factory, lmsgclass->msgdef); -} - const upb_msgdef *lupb_msgclass_getmsgdef(const lupb_msgclass *lmsgclass) { return lmsgclass->msgdef; } @@ -547,12 +515,6 @@ static int lupb_array_checkindex(lua_State *L, int narg, uint32_t max) { return n - 1; /* Lua uses 1-based indexing. :( */ } -static int lupb_array_gc(lua_State *L) { - lupb_array *larray = lupb_array_check(L, 1); - upb_array_uninit(larray->arr); - return 0; -} - /* lupb_array Public API */ static int lupb_array_new(lua_State *L) { @@ -568,11 +530,9 @@ static int lupb_array_new(lua_State *L) { lupb_uservalseti(L, -1, ARRAY_MSGCLASS_INDEX, 1); /* GC-root lmsgclass. */ } - larray = - lupb_newuserdata(L, sizeof(*larray) + upb_array_sizeof(type), LUPB_ARRAY); + larray = lupb_newuserdata(L, sizeof(*larray), LUPB_ARRAY); larray->lmsgclass = lmsgclass; - larray->arr = ADD_BYTES(larray, sizeof(*larray)); - upb_array_init(larray->arr, type, lupb_alloc_get(L)); + larray->arr = upb_array_new(type, lupb_arena_get(L)); return 1; } @@ -614,7 +574,6 @@ static int lupb_array_len(lua_State *L) { } static const struct luaL_Reg lupb_array_mm[] = { - {"__gc", lupb_array_gc}, {"__index", lupb_array_index}, {"__len", lupb_array_len}, {"__newindex", lupb_array_newindex}, @@ -681,12 +640,6 @@ static upb_msgval lupb_map_typecheck(lua_State *L, int narg, int msg, return upb_msgval_map(map); } -static int lupb_map_gc(lua_State *L) { - lupb_map *lmap = lupb_map_check(L, 1); - upb_map_uninit(lmap->map); - return 0; -} - /* lupb_map Public API */ /** @@ -707,9 +660,7 @@ static int lupb_map_new(lua_State *L) { value_type = UPB_TYPE_MESSAGE; } - lmap = lupb_newuserdata( - L, sizeof(*lmap) + upb_map_sizeof(key_type, value_type), LUPB_MAP); - lmap->map = ADD_BYTES(lmap, sizeof(*lmap)); + lmap = lupb_newuserdata(L, sizeof(*lmap), LUPB_MAP); if (value_type == UPB_TYPE_MESSAGE) { value_lmsgclass = lupb_msgclass_check(L, 2); @@ -717,7 +668,7 @@ static int lupb_map_new(lua_State *L) { } lmap->value_lmsgclass = value_lmsgclass; - upb_map_init(lmap->map, key_type, value_type, lupb_alloc_get(L)); + lmap->map = upb_map_new(key_type, value_type, lupb_arena_get(L)); return 1; } @@ -858,7 +809,6 @@ static int lupb_map_pairs(lua_State *L) { /* upb_mapiter ]]] */ static const struct luaL_Reg lupb_map_mm[] = { - {"__gc", lupb_map_gc}, {"__index", lupb_map_index}, {"__len", lupb_map_len}, {"__newindex", lupb_map_newindex}, @@ -912,6 +862,13 @@ const upb_msg *lupb_msg_checkmsg(lua_State *L, int narg, return lmsg->msg; } +upb_msg *lupb_msg_checkmsg2(lua_State *L, int narg, + const upb_msglayout **layout) { + lupb_msg *lmsg = lupb_msg_check(L, narg); + *layout = lmsg->lmsgclass->layout; + return lmsg->msg; +} + const upb_msgdef *lupb_msg_checkdef(lua_State *L, int narg) { return lupb_msg_check(L, narg)->lmsgclass->msgdef; } @@ -958,12 +915,6 @@ int lupb_msg_pushref(lua_State *L, int msgclass, upb_msg *msg) { return 1; } -static int lupb_msg_gc(lua_State *L) { - lupb_msg *lmsg = lupb_msg_check(L, 1); - upb_msg_uninit(lmsg->msg, lmsg->lmsgclass->layout); - return 0; -} - /* lupb_msg Public API */ /** @@ -974,12 +925,10 @@ static int lupb_msg_gc(lua_State *L) { */ static int lupb_msg_pushnew(lua_State *L, int narg) { const lupb_msgclass *lmsgclass = lupb_msgclass_check(L, narg); - size_t size = sizeof(lupb_msg) + upb_msg_sizeof(lmsgclass->layout); - lupb_msg *lmsg = lupb_newuserdata(L, size, LUPB_MSG); + lupb_msg *lmsg = lupb_newuserdata(L, sizeof(lupb_msg), LUPB_MSG); lmsg->lmsgclass = lmsgclass; - lmsg->msg = upb_msg_init( - ADD_BYTES(lmsg, sizeof(*lmsg)), lmsgclass->layout, lupb_alloc_get(L)); + lmsg->msg = upb_msg_new(lmsgclass->layout, lupb_arena_get(L)); lupb_uservalseti(L, -1, LUPB_MSG_MSGCLASSINDEX, narg); @@ -1070,7 +1019,6 @@ static int lupb_msg_newindex(lua_State *L) { } static const struct luaL_Reg lupb_msg_mm[] = { - {"__gc", lupb_msg_gc}, {"__index", lupb_msg_index}, {"__newindex", lupb_msg_newindex}, {NULL, NULL} @@ -1096,5 +1044,5 @@ void lupb_msg_registertypes(lua_State *L) { lupb_register_type(L, LUPB_MAP, NULL, lupb_map_mm); lupb_register_type(L, LUPB_MSG, NULL, lupb_msg_mm); - lupb_alloc_initsingleton(L); + lupb_arena_initsingleton(L); } diff --git a/upb/bindings/lua/upb.h b/upb/bindings/lua/upb.h index ea2910a..52bc5a2 100644 --- a/upb/bindings/lua/upb.h +++ b/upb/bindings/lua/upb.h @@ -131,11 +131,12 @@ int lupb_arena_new(lua_State *L); int lupb_msg_pushref(lua_State *L, int msgclass, void *msg); const upb_msg *lupb_msg_checkmsg(lua_State *L, int narg, const lupb_msgclass *lmsgclass); +upb_msg *lupb_msg_checkmsg2(lua_State *L, int narg, + const upb_msglayout **layout); const lupb_msgclass *lupb_msgclass_check(lua_State *L, int narg); const upb_msglayout *lupb_msgclass_getlayout(lua_State *L, int narg); const upb_msgdef *lupb_msgclass_getmsgdef(const lupb_msgclass *lmsgclass); -const upb_handlers *lupb_msgclass_getmergehandlers(lua_State *L, int narg); upb_msgfactory *lupb_msgclass_getfactory(const lupb_msgclass *lmsgclass); void lupb_msg_registertypes(lua_State *L); diff --git a/upb/bindings/lua/upb/pb.c b/upb/bindings/lua/upb/pb.c index 1d56066..466e8f7 100644 --- a/upb/bindings/lua/upb/pb.c +++ b/upb/bindings/lua/upb/pb.c @@ -6,135 +6,56 @@ */ #include "upb/bindings/lua/upb.h" -#include "upb/pb/decoder.h" -#include "upb/pb/encoder.h" +#include "upb/decode.h" +#include "upb/encode.h" #define LUPB_PBDECODERMETHOD "lupb.pb.decodermethod" -static int lupb_pb_strtomessage(lua_State *L) { +static int lupb_pb_decode(lua_State *L) { size_t len; upb_status status = UPB_STATUS_INIT; - const char *pb = lua_tolstring(L, 1, &len); - const upb_msglayout *layout = lua_touserdata(L, lua_upvalueindex(1)); - const upb_pbdecodermethod *method = lua_touserdata(L, lua_upvalueindex(2)); - const upb_handlers *handlers = upb_pbdecodermethod_desthandlers(method); - - upb_arena *msg_arena; + const upb_msglayout *layout; + upb_msg *msg = lupb_msg_checkmsg2(L, 1, &layout); + const char *pb = lua_tolstring(L, 2, &len); + upb_stringview buf = upb_stringview_make(pb, len); upb_env env; - upb_sink sink; - upb_pbdecoder *decoder; - void *msg; - - lupb_arena_new(L); - msg_arena = lupb_arena_check(L, -1); - msg = upb_msg_new(layout, upb_arena_alloc(msg_arena)); upb_env_init(&env); upb_env_reporterrorsto(&env, &status); - upb_sink_reset(&sink, handlers, msg); - decoder = upb_pbdecoder_create(&env, method, &sink); - upb_bufsrc_putbuf(pb, len, upb_pbdecoder_input(decoder)); + upb_decode(buf, msg, (const void*)layout, &env); /* Free resources before we potentially bail on error. */ upb_env_uninit(&env); lupb_checkstatus(L, &status); - /* References the arena at the top of the stack. */ - lupb_msg_pushref(L, lua_upvalueindex(3), msg); - return 1; + return 0; } -static int lupb_pb_messagetostr(lua_State *L) { - const lupb_msgclass *lmsgclass = lua_touserdata(L, lua_upvalueindex(1)); - const upb_msg *msg = lupb_msg_checkmsg(L, 1, lmsgclass); - const upb_visitorplan *vp = lua_touserdata(L, lua_upvalueindex(2)); - const upb_handlers *encode_handlers = lua_touserdata(L, lua_upvalueindex(3)); - +static int lupb_pb_encode(lua_State *L) { + const upb_msglayout *layout; + const upb_msg *msg = lupb_msg_checkmsg2(L, 1, &layout); upb_env env; - upb_bufsink *bufsink; - upb_bytessink *bytessink; - upb_pb_encoder *encoder; - upb_visitor *visitor; - const char *buf; - size_t len; + size_t size; upb_status status = UPB_STATUS_INIT; + char *result; upb_env_init(&env); upb_env_reporterrorsto(&env, &status); - bufsink = upb_bufsink_new(&env); - bytessink = upb_bufsink_sink(bufsink); - encoder = upb_pb_encoder_create(&env, encode_handlers, bytessink); - visitor = upb_visitor_create(&env, vp, upb_pb_encoder_input(encoder)); - upb_visitor_visitmsg(visitor, msg); - buf = upb_bufsink_getdata(bufsink, &len); - lua_pushlstring(L, buf, len); + result = upb_encode(msg, (const void*)layout, &env, &size); /* Free resources before we potentially bail on error. */ upb_env_uninit(&env); lupb_checkstatus(L, &status); + lua_pushlstring(L, result, size); return 1; } -static int lupb_pb_makestrtomsgdecoder(lua_State *L) { - const upb_msglayout *layout = lupb_msgclass_getlayout(L, 1); - const upb_handlers *handlers = lupb_msgclass_getmergehandlers(L, 1); - const upb_pbdecodermethod *m; - - upb_pbdecodermethodopts opts; - upb_pbdecodermethodopts_init(&opts, handlers); - - m = upb_pbdecodermethod_new(&opts, &m); - - /* Push upvalues for the closure. */ - lua_pushlightuserdata(L, (void*)layout); - lua_pushlightuserdata(L, (void*)m); - lua_pushvalue(L, 1); - - /* Upvalue for the closure, only to keep the decodermethod alive. */ - lupb_refcounted_pushnewrapper( - L, upb_pbdecodermethod_upcast(m), LUPB_PBDECODERMETHOD, &m); - - lua_pushcclosure(L, &lupb_pb_strtomessage, 4); - - return 1; /* The decoder closure. */ -} - -static int lupb_pb_makemsgtostrencoder(lua_State *L) { - const lupb_msgclass *lmsgclass = lupb_msgclass_check(L, 1); - const upb_msgdef *md = lupb_msgclass_getmsgdef(lmsgclass); - upb_msgfactory *factory = lupb_msgclass_getfactory(lmsgclass); - const upb_handlers *encode_handlers; - const upb_visitorplan *vp; - - encode_handlers = upb_pb_encoder_newhandlers(md, &encode_handlers); - vp = upb_msgfactory_getvisitorplan(factory, encode_handlers); - - /* Push upvalues for the closure. */ - lua_pushlightuserdata(L, (void*)lmsgclass); - lua_pushlightuserdata(L, (void*)vp); - lua_pushlightuserdata(L, (void*)encode_handlers); - - /* Upvalues for the closure, only to keep the other upvalues alive. */ - lua_pushvalue(L, 1); - lupb_refcounted_pushnewrapper( - L, upb_handlers_upcast(encode_handlers), LUPB_PBDECODERMETHOD, &encode_handlers); - - lua_pushcclosure(L, &lupb_pb_messagetostr, 5); - - return 1; /* The decoder closure. */ -} - -static const struct luaL_Reg decodermethod_mm[] = { - {"__gc", lupb_refcounted_gc}, - {NULL, NULL} -}; - static const struct luaL_Reg toplevel_m[] = { - {"MakeStringToMessageDecoder", lupb_pb_makestrtomsgdecoder}, - {"MakeMessageToStringEncoder", lupb_pb_makemsgtostrencoder}, + {"decode", lupb_pb_decode}, + {"encode", lupb_pb_encode}, {NULL, NULL} }; @@ -144,7 +65,5 @@ int luaopen_upb_pb_c(lua_State *L) { return 1; } - lupb_register_type(L, LUPB_PBDECODERMETHOD, NULL, decodermethod_mm); - return 1; } -- cgit v1.2.3