starwing / lua-protobuf

A Lua module to work with Google protobuf
MIT License
1.75k stars 387 forks source link

add pack unpack message api #208

Closed changnet closed 2 years ago

changnet commented 2 years ago

相关讨论 #203

local name = "ilise"
local id = 10
local email = "888888888@github.com"
local phone = {1 = {type = "MOBILE",number = "12312341234"},2 = {type = "WORK",number = "45645674567"}}}
local buffer_person = pb.pack_msg("tutorial.Person", name, id, email, phone)
name, id, email, phone = pb.unpack_msg("tutorial.Person", buffer_person)

一些简单的测试结果 https://github.com/changnet/protobuf-test

coveralls commented 2 years ago

Coverage Status

Coverage increased (+0.02%) to 99.676% when pulling c55c07c497d0f993e05c45e09924c8ac8be756ca on changnet:master into 6719a107bda7419e63f0ecaf67e69f540b3457aa on starwing:master.

starwing commented 2 years ago

你这样直接加了field的排序真的没问题么……?都没有处理pb_newfield/pb_delfield啊……先不说多分配一块内存了(这个好像都不是很重要了),这种排序,添加删除field的时候怎么维护……

changnet commented 2 years ago

确实漏了 pb_newfield/pb_delfield 的处理,添加删除field需要把已有的的排序free掉,用到时再重新排序

内存分配这个,似乎没法避免

starwing commented 2 years ago

这个还是要全局想想,我之前不太想做这个的一个主要原因就是内存分配。另外排序可能问题不大,目前唯一会修改的地方是load,load结束再排序就可以。还有一个问题,扩展extension是会插入到其他类型的,如果每次都排序,那么排序复杂度是$O(n^2lgn)$,这肯定是不能接受的。

changnet commented 2 years ago

内存分配和排序这个我觉得可以接受,因为一般的使用场景,proto文件加载后就很少会变化。偶尔更新一次proto进行重排消耗不算大(比如游戏里进行热更新)。这个排序是用到时再排的,如果有什么特殊的使用场景,避免调用pack、unpack,那即不会分配内存,也不会排序

extension这个,我从来没用过,要花点时间去看一下才知道

starwing commented 2 years ago

那倒是可以。可以考虑比如说如果有field修改就置位一个标记,如果需要排号序的发现有标记就排个序,这样应该可以。

changnet commented 2 years ago

补上了newfield delfield的处理。newfield、delfield是根据field_sort字段是否为NULL判断是否存在排序,而字段的排序是调用pack或者unpack时才会执行,应该不会对加载pb文件造成影响

不过因为字段的排序是调用pack或者unpack时才会执行,也会造成pb_sortfield执行时,需要把一个const pb_Type*强转为pb_Type*来进行字段排序,因为现有的接口不方便根据字段名取pb_Type*类型的指针

starwing commented 2 years ago

我看到很多地方调用了gettop,不过其实没有必要,只要允许负索引,然后实现一个lpb_relindex函数就好了,具体实现是,如果索引是正的或者小于等于LUA_REGISTRYINDEX,那么保持不变,否则,减去一个传入的当前栈上多余的元素数量即可。这个技巧是我在其他项目的源码里面学会的,能够少一次跨dll调用(并且减少某些函数长度……我如果有可能,我都是希望函数最多25行的)

starwing commented 2 years ago

还有就是,目前代码的函数名是Lua风格的——也就是说,除了函数名前缀,所有字母小写并且不用下划线。说实在的有点不太好认,但是短,而且强迫你想又短又好认的函数名(比如lpbE_pack就足够了,或者lpb_packmsg也行)

starwing commented 2 years ago

说到风格,else不换行也是(刚开始我也不太习惯,不过现在觉得不换行好像更好看一点)

changnet commented 2 years ago

关于调用了gettop这个,之前的做法是用pushvalue来复制一份到栈顶,所以可以传负索引或者说不传索引,默认使用-1。现在用gettop是因为想省去做这个复制。这两种做法,我确实也有点疑问它们差别有多大,最终测出来的结果是差别太小,看不出来。如果想用pushvalue的方式,我可以改一下

命名这个,是按上一版本写的。我原本考虑是用pack、unpack,但现在pb.pack、pb.unpack这两函数是被占用了,所以之前有问一下是否考虑把现有的pack、unpack接口放回buffer和slice模块,然后这两个接口用pack、unpack。如果不改,那我可以按现有的风格改成lpbE_packmsg、lpbD_unpackmsg

换行风格这个,个人喜好吧,等上面的问题确定了我一起改一下

starwing commented 2 years ago

关于调用了gettop这个,之前的做法是用pushvalue来复制一份到栈顶,所以可以传负索引或者说不传索引,默认使用-1。现在用gettop是因为想省去做这个复制。这两种做法,我确实也有点疑问它们差别有多大,最终测出来的结果是差别太小,看不出来。如果想用pushvalue的方式,我可以改一下

啊,不是这个意思,首先其实差别不大,其次pushvalue的方式还是有损耗嘛,我说的意思是这样的:

static int lpb_relindex(lua_State *L, int idx, int onstack)
{ return idx >= 0 || idx <= LUA_REGISTRYINDEX ? idx : idx - onstack; }

static void dosomething(lua_State *L, int idx) {
    // 假设现在栈上有两个元素:
    lua_pushliteral(L, "key");
    lua_pushnil(L);
    // 现在想用idx了:
    lua_settable(L, lpb_relindex(L, idx, 2));
}

这样就完全不需要gettop啊,pushvalue啊什么的了。

命名这个,是按上一版本写的。我原本考虑是用pack、unpack,但现在pb.pack、pb.unpack这两函数是被占用了,所以之前有问一下是否考虑把现有的pack、unpack接口放回buffer和slice模块,然后这两个接口用pack、unpack。如果不改,那我可以按现有的风格改成lpbE_packmsg、lpbD_unpackmsg

就直接换掉好了,好像的确没啥人用。

changnet commented 2 years ago

static int lpb_relindex(lua_State *L, int idx, int onstack)需要另一个参数onstack,要么就用现在的gettop来取,要么在对应的函数再加一个参数,或者放到lpb_Env *e中来维护,感觉都不算优雅。有什么其他好办法么?

starwing commented 2 years ago

static int lpb_relindex(lua_State *L, int idx, int onstack)需要另一个参数onstack,要么就用现在的gettop来取,要么在对应的函数再加一个参数,或者放到lpb_Env *e中来维护,感觉都不算优雅。有什么其他好办法么?

并不需要哇……你还是没理解这个方法,当你调用一个传入了idx的函数的时候,唯一会导致idx出错的情况是你动了栈(比如push了或者pop了啥),你在这个函数里面当然是知道自己干了啥的,所以你自己数函数里面怎么个动法,然后做一个常量传给lpb_relindex就行了,比如你知道在之前的操作里你往栈上推了5个值,你就传5,如果你推了3个值,你就传3,如果你pop了2个值(并且这两个值都是在idx之后的)那么你就传-2。这个函数就可以帮你算出在这个位置正确的idx值。

比如上面的例子,假设你调用dosomething(L, -2),当函数运行到lua_settable的时候,作为dosomething函数的作者你当然知道,从这个函数开始到这个地点,你对栈的操作是“push了两个值”,所以你传了2,然后idx就变成了 (-2)-2 也就是 -4,正好是你在push了两个值以后,idx指向的那个位置的正确索引。

starwing commented 2 years ago

我用一个具体的例子给你看怎么改好了,比如lpbE_map这个函数,改完以后长这样:

static void lpbE_map(lpb_Env *e, const pb_Field *f, int idx) {
    lua_State *L = e->L;
    const pb_Field *kf = pb_field(f->type, 1);
    const pb_Field *vf = pb_field(f->type, 2);
    if (kf == NULL || vf == NULL) return;
    lpb_checktable(L, f, idx); // 这里没推任何值,直接用idx
    lua_pushnil(L); // 这里push了一个值,所以
    while (lua_next(L, lpb_relindex(idx, 1))) { // 这里传1
        size_t len;
        pb_addvarint32(e->b, pb_pair(f->number, PB_TBYTES));
        len = pb_bufflen(e->b);
        lpbE_tagfield(e, kf, 1, -2); // 这里可以传负数了!
        lpbE_tagfield(e, vf, 1, -1);
        lpb_addlength(L, e->b, len);
        lua_pop(L, 1);
    }
}
changnet commented 2 years ago
  1. 使用lpb_relindex替换原有的gettop方式
  2. 接口名改成pb.pack、pb.unpack
  3. 整理了代码风格
starwing commented 2 years ago

多谢~ 有了sortfields很多事情就都比较好办了~

starwing commented 2 years ago

不过说起来你好像忘了改文档🤣

changnet commented 2 years ago

确实没改文档。麻烦你那边改一下