perf(api): avoid spurious allocations when converting small objects

bfredl · bfredl · commit 705e8f10ac83 · 2021-08-28T16:52:01.000+02:00
Converter functions use a heap-allocated stack to handle complex
nested objects. However, these are often called with simple,
primitive values like integers or bools wrapped in an Object.
Avoid the memory allocation in this case using kvec_withinit_t
diff --git a/src/nvim/api/private/helpers.c b/src/nvim/api/private/helpers.c
@@ -37,7 +37,7 @@
 
 /// Helper structure for vim_to_object
 typedef struct {
-  kvec_t(Object) stack;  ///< Object stack.
+  kvec_withinit_t(Object, 2) stack;  ///< Object stack.
 } EncodedData;
 
 #ifdef INCLUDE_GENERATED_DECLARATIONS
@@ -418,28 +418,25 @@ void set_option_to(uint64_t channel_id, void *to, int type,
 #define TYPVAL_ENCODE_ALLOW_SPECIALS false
 
 #define TYPVAL_ENCODE_CONV_NIL(tv) \
-    kv_push(edata->stack, NIL)
+    kvi_push(edata->stack, NIL)
 
 #define TYPVAL_ENCODE_CONV_BOOL(tv, num) \
-    kv_push(edata->stack, BOOLEAN_OBJ((Boolean)(num)))
+    kvi_push(edata->stack, BOOLEAN_OBJ((Boolean)(num)))
 
 #define TYPVAL_ENCODE_CONV_NUMBER(tv, num) \
-    kv_push(edata->stack, INTEGER_OBJ((Integer)(num)))
+    kvi_push(edata->stack, INTEGER_OBJ((Integer)(num)))
 
 #define TYPVAL_ENCODE_CONV_UNSIGNED_NUMBER TYPVAL_ENCODE_CONV_NUMBER
 
 #define TYPVAL_ENCODE_CONV_FLOAT(tv, flt) \
-    kv_push(edata->stack, FLOAT_OBJ((Float)(flt)))
+    kvi_push(edata->stack, FLOAT_OBJ((Float)(flt)))
 
 #define TYPVAL_ENCODE_CONV_STRING(tv, str, len) \
     do { \
       const size_t len_ = (size_t)(len); \
       const char *const str_ = (const char *)(str); \
       assert(len_ == 0 || str_ != NULL); \
-      kv_push(edata->stack, STRING_OBJ(((String) { \
-        .data = xmemdupz((len_?str_:""), len_), \
-        .size = len_ \
-      }))); \
+      kvi_push(edata->stack, STRING_OBJ(cbuf_to_string((len_?str_:""), len_))); \
     } while (0)
 
 #define TYPVAL_ENCODE_CONV_STR_STRING TYPVAL_ENCODE_CONV_STRING
@@ -458,17 +455,17 @@ void set_option_to(uint64_t channel_id, void *to, int type,
 #define TYPVAL_ENCODE_CONV_FUNC_END(tv)
 
 #define TYPVAL_ENCODE_CONV_EMPTY_LIST(tv) \
-    kv_push(edata->stack, ARRAY_OBJ(((Array) { .capacity = 0, .size = 0 })))
+    kvi_push(edata->stack, ARRAY_OBJ(((Array) { .capacity = 0, .size = 0 })))
 
 #define TYPVAL_ENCODE_CONV_EMPTY_DICT(tv, dict) \
-    kv_push(edata->stack, \
-            DICTIONARY_OBJ(((Dictionary) { .capacity = 0, .size = 0 })))
+    kvi_push(edata->stack, \
+             DICTIONARY_OBJ(((Dictionary) { .capacity = 0, .size = 0 })))
 
 static inline void typval_encode_list_start(EncodedData *const edata,
                                             const size_t len)
   FUNC_ATTR_ALWAYS_INLINE FUNC_ATTR_NONNULL_ALL
 {
-  kv_push(edata->stack, ARRAY_OBJ(((Array) {
+  kvi_push(edata->stack, ARRAY_OBJ(((Array) {
     .capacity = len,
     .size = 0,
     .items = xmalloc(len * sizeof(*((Object)OBJECT_INIT).data.array.items)),
@@ -510,7 +507,7 @@ static inline void typval_encode_dict_start(EncodedData *const edata,
                                             const size_t len)
   FUNC_ATTR_ALWAYS_INLINE FUNC_ATTR_NONNULL_ALL
 {
-  kv_push(edata->stack, DICTIONARY_OBJ(((Dictionary) {
+  kvi_push(edata->stack, DICTIONARY_OBJ(((Dictionary) {
     .capacity = len,
     .size = 0,
     .items = xmalloc(len * sizeof(
@@ -618,14 +615,15 @@ static inline void typval_encode_dict_end(EncodedData *const edata)
 /// @return The converted value
 Object vim_to_object(typval_T *obj)
 {
-  EncodedData edata = { .stack = KV_INITIAL_VALUE };
+  EncodedData edata;
+  kvi_init(edata.stack);
   const int evo_ret = encode_vim_to_object(&edata, obj,
                                            "vim_to_object argument");
   (void)evo_ret;
   assert(evo_ret == OK);
   Object ret = kv_A(edata.stack, 0);
   assert(kv_size(edata.stack) == 1);
-  kv_destroy(edata.stack);
+  kvi_destroy(edata.stack);
   return ret;
 }
 
diff --git a/src/nvim/lua/converter.c b/src/nvim/lua/converter.c
@@ -196,8 +196,9 @@ bool nlua_pop_typval(lua_State *lstate, typval_T *ret_tv)
 {
   bool ret = true;
   const int initial_size = lua_gettop(lstate);
-  kvec_t(TVPopStackItem) stack = KV_INITIAL_VALUE;
-  kv_push(stack, ((TVPopStackItem) { ret_tv, false, false, 0 }));
+  kvec_withinit_t(TVPopStackItem, 2) stack = KV_INITIAL_VALUE;
+  kvi_init(stack);
+  kvi_push(stack, ((TVPopStackItem) { ret_tv, false, false, 0 }));
   while (ret && kv_size(stack)) {
     if (!lua_checkstack(lstate, lua_gettop(lstate) + 3)) {
       emsgf(_("E1502: Lua failed to grow stack to %i"), lua_gettop(lstate) + 3);
@@ -234,7 +235,7 @@ bool nlua_pop_typval(lua_State *lstate, typval_T *ret_tv)
             tv_list_append_owned_tv(kv_pair, (typval_T) {
               .v_type = VAR_UNKNOWN,
             });
-            kv_push(stack, cur);
+            kvi_push(stack, cur);
             tv_list_append_list(cur.tv->vval.v_list, kv_pair);
             cur = (TVPopStackItem) {
               .tv = TV_LIST_ITEM_TV(tv_list_last(kv_pair)),
@@ -247,7 +248,7 @@ bool nlua_pop_typval(lua_State *lstate, typval_T *ret_tv)
             if (tv_dict_add(cur.tv->vval.v_dict, di) == FAIL) {
               abort();
             }
-            kv_push(stack, cur);
+            kvi_push(stack, cur);
             cur = (TVPopStackItem) { &di->di_tv, false, false, 0 };
           }
         } else {
@@ -265,7 +266,7 @@ bool nlua_pop_typval(lua_State *lstate, typval_T *ret_tv)
         tv_list_append_owned_tv(cur.tv->vval.v_list, (typval_T) {
           .v_type = VAR_UNKNOWN,
         });
-        kv_push(stack, cur);
+        kvi_push(stack, cur);
         // TODO(ZyX-I): Use indexes, here list item *will* be reallocated.
         cur = (TVPopStackItem) {
           .tv = TV_LIST_ITEM_TV(tv_list_last(cur.tv->vval.v_list)),
@@ -343,7 +344,7 @@ bool nlua_pop_typval(lua_State *lstate, typval_T *ret_tv)
             if (table_props.maxidx != 0) {
               cur.container = true;
               cur.idx = lua_gettop(lstate);
-              kv_push(stack, cur);
+              kvi_push(stack, cur);
             }
             break;
           }
@@ -373,7 +374,7 @@ bool nlua_pop_typval(lua_State *lstate, typval_T *ret_tv)
               }
               cur.container = true;
               cur.idx = lua_gettop(lstate);
-              kv_push(stack, cur);
+              kvi_push(stack, cur);
               lua_pushnil(lstate);
             }
             break;
@@ -434,7 +435,7 @@ bool nlua_pop_typval(lua_State *lstate, typval_T *ret_tv)
       lua_pop(lstate, 1);
     }
   }
-  kv_destroy(stack);
+  kvi_destroy(stack);
   if (!ret) {
     tv_clear(ret_tv);
     *ret_tv = (typval_T) {
@@ -1060,15 +1061,16 @@ Object nlua_pop_Object(lua_State *const lstate, bool ref, Error *const err)
 {
   Object ret = NIL;
   const int initial_size = lua_gettop(lstate);
-  kvec_t(ObjPopStackItem) stack = KV_INITIAL_VALUE;
-  kv_push(stack, ((ObjPopStackItem) { &ret, false }));
+  kvec_withinit_t(ObjPopStackItem, 2) stack = KV_INITIAL_VALUE;
+  kvi_init(stack);
+  kvi_push(stack, ((ObjPopStackItem) { &ret, false }));
   while (!ERROR_SET(err) && kv_size(stack)) {
-    if (!lua_checkstack(lstate, lua_gettop(lstate) + 3)) {
-      api_set_error(err, kErrorTypeException, "Lua failed to grow stack");
-      break;
-    }
     ObjPopStackItem cur = kv_pop(stack);
     if (cur.container) {
+      if (!lua_checkstack(lstate, lua_gettop(lstate) + 3)) {
+        api_set_error(err, kErrorTypeException, "Lua failed to grow stack");
+        break;
+      }
       if (cur.obj->type == kObjectTypeDictionary) {
         // stack: …, dict, key
         if (cur.obj->data.dictionary.size
@@ -1095,7 +1097,7 @@ Object nlua_pop_Object(lua_State *const lstate, bool ref, Error *const err)
             .data = xmemdupz(s, len),
             .size = len,
           };
-          kv_push(stack, cur);
+          kvi_push(stack, cur);
           cur = (ObjPopStackItem) {
             .obj = &cur.obj->data.dictionary.items[idx].value,
             .container = false,
@@ -1117,7 +1119,7 @@ Object nlua_pop_Object(lua_State *const lstate, bool ref, Error *const err)
           lua_pop(lstate, 2);
           continue;
         }
-        kv_push(stack, cur);
+        kvi_push(stack, cur);
         cur = (ObjPopStackItem) {
           .obj = &cur.obj->data.array.items[idx],
           .container = false,
@@ -1169,7 +1171,7 @@ Object nlua_pop_Object(lua_State *const lstate, bool ref, Error *const err)
                           sizeof(cur.obj->data.array.items[0]));
               cur.obj->data.array.capacity = table_props.maxidx;
               cur.container = true;
-              kv_push(stack, cur);
+              kvi_push(stack, cur);
             }
             break;
           }
@@ -1185,7 +1187,7 @@ Object nlua_pop_Object(lua_State *const lstate, bool ref, Error *const err)
                           sizeof(cur.obj->data.dictionary.items[0]));
               cur.obj->data.dictionary.capacity = table_props.string_keys_num;
               cur.container = true;
-              kv_push(stack, cur);
+              kvi_push(stack, cur);
               lua_pushnil(lstate);
             }
             break;
@@ -1239,7 +1241,7 @@ Object nlua_pop_Object(lua_State *const lstate, bool ref, Error *const err)
       lua_pop(lstate, 1);
     }
   }
-  kv_destroy(stack);
+  kvi_destroy(stack);
   if (ERROR_SET(err)) {
     api_free_object(ret);
     ret = NIL;
diff --git a/src/nvim/msgpack_rpc/helpers.c b/src/nvim/msgpack_rpc/helpers.c
@@ -86,8 +86,9 @@ bool msgpack_rpc_to_object(const msgpack_object *const obj, Object *const arg)
   FUNC_ATTR_NONNULL_ALL
 {
   bool ret = true;
-  kvec_t(MPToAPIObjectStackItem) stack = KV_INITIAL_VALUE;
-  kv_push(stack, ((MPToAPIObjectStackItem) {
+  kvec_withinit_t(MPToAPIObjectStackItem, 2) stack = KV_INITIAL_VALUE;
+  kvi_init(stack);
+  kvi_push(stack, ((MPToAPIObjectStackItem) {
     .mobj = obj,
     .aobj = arg,
     .container = false,
@@ -155,7 +156,7 @@ bool msgpack_rpc_to_object(const msgpack_object *const obj, Object *const arg)
             const size_t idx = cur.idx;
             cur.idx++;
             kv_last(stack) = cur;
-            kv_push(stack, ((MPToAPIObjectStackItem) {
+            kvi_push(stack, ((MPToAPIObjectStackItem) {
               .mobj = &cur.mobj->via.array.ptr[idx],
               .aobj = &cur.aobj->data.array.items[idx],
               .container = false,
@@ -209,7 +210,7 @@ bool msgpack_rpc_to_object(const msgpack_object *const obj, Object *const arg)
               }
             }
             if (ret) {
-              kv_push(stack, ((MPToAPIObjectStackItem) {
+              kvi_push(stack, ((MPToAPIObjectStackItem) {
                 .mobj = &cur.mobj->via.map.ptr[idx].val,
                 .aobj = &cur.aobj->data.dictionary.items[idx].value,
                 .container = false,
@@ -265,7 +266,7 @@ bool msgpack_rpc_to_object(const msgpack_object *const obj, Object *const arg)
       (void)kv_pop(stack);
     }
   }
-  kv_destroy(stack);
+  kvi_destroy(stack);
   return ret;
 }
 
@@ -375,8 +376,9 @@ typedef struct {
 void msgpack_rpc_from_object(const Object result, msgpack_packer *const res)
   FUNC_ATTR_NONNULL_ARG(2)
 {
-  kvec_t(APIToMPObjectStackItem) stack = KV_INITIAL_VALUE;
-  kv_push(stack, ((APIToMPObjectStackItem) { &result, false, 0 }));
+  kvec_withinit_t(APIToMPObjectStackItem, 2) stack = KV_INITIAL_VALUE;
+  kvi_init(stack);
+  kvi_push(stack, ((APIToMPObjectStackItem) { &result, false, 0 }));
   while (kv_size(stack)) {
     APIToMPObjectStackItem cur = kv_last(stack);
     STATIC_ASSERT(kObjectTypeWindow == kObjectTypeBuffer + 1
@@ -428,7 +430,7 @@ void msgpack_rpc_from_object(const Object result, msgpack_packer *const res)
             const size_t idx = cur.idx;
             cur.idx++;
             kv_last(stack) = cur;
-            kv_push(stack, ((APIToMPObjectStackItem) {
+            kvi_push(stack, ((APIToMPObjectStackItem) {
               .aobj = &cur.aobj->data.array.items[idx],
               .container = false,
             }));
@@ -451,7 +453,7 @@ void msgpack_rpc_from_object(const Object result, msgpack_packer *const res)
             kv_last(stack) = cur;
             msgpack_rpc_from_string(cur.aobj->data.dictionary.items[idx].key,
                                     res);
-            kv_push(stack, ((APIToMPObjectStackItem) {
+            kvi_push(stack, ((APIToMPObjectStackItem) {
               .aobj = &cur.aobj->data.dictionary.items[idx].value,
               .container = false,
             }));
@@ -468,7 +470,7 @@ void msgpack_rpc_from_object(const Object result, msgpack_packer *const res)
       (void)kv_pop(stack);
     }
   }
-  kv_destroy(stack);
+  kvi_destroy(stack);
 }
 
 void msgpack_rpc_from_array(Array result, msgpack_packer *res)
