perf(ui): eliminate spurious memory allocations for hl_attr_define event

7 files changed, 58 insertions, 38 deletions

diff --git a/src/nvim/api/private/helpers.h b/src/nvim/api/private/helpers.h
index a4348d8b44..224ce41aae 100644
--- a/src/nvim/api/private/helpers.h
+++ b/src/nvim/api/private/helpers.h
@@ -86,6 +86,12 @@
   name.capacity = maxsize; \
   name.items = name##__items; \
 
+#define MAXSIZE_TEMP_DICT(name, maxsize) \
+  Dictionary name = ARRAY_DICT_INIT; \
+  KeyValuePair name##__items[maxsize]; \
+  name.capacity = maxsize; \
+  name.items = name##__items; \
+
 #define cbuf_as_string(d, s) ((String) { .data = d, .size = s })
 
 #define STATIC_CSTR_AS_STRING(s) ((String) { .data = s, .size = sizeof(s) - 1 })
diff --git a/src/nvim/api/ui.c b/src/nvim/api/ui.c
index 54ce838b9b..aa7bed1132 100644
--- a/src/nvim/api/ui.c
+++ b/src/nvim/api/ui.c
@@ -748,8 +748,10 @@ static void remote_ui_hl_attr_define(UI *ui, Integer id, HlAttrs rgb_attrs, HlAt
   UIData *data = ui->data;
   Array args = data->call_buf;
   ADD_C(args, INTEGER_OBJ(id));
-  ADD_C(args, DICTIONARY_OBJ(hlattrs2dict(rgb_attrs, true)));
-  ADD_C(args, DICTIONARY_OBJ(hlattrs2dict(cterm_attrs, false)));
+  MAXSIZE_TEMP_DICT(rgb, 16);
+  MAXSIZE_TEMP_DICT(cterm, 16);
+  ADD_C(args, DICTIONARY_OBJ(hlattrs2dict(&rgb, rgb_attrs, true)));
+  ADD_C(args, DICTIONARY_OBJ(hlattrs2dict(&cterm, cterm_attrs, false)));
 
   if (ui->ui_ext[kUIHlState]) {
     ADD_C(args, ARRAY_OBJ(info));
@@ -758,9 +760,6 @@ static void remote_ui_hl_attr_define(UI *ui, Integer id, HlAttrs rgb_attrs, HlAt
   }
 
   push_call(ui, "hl_attr_define", args);
-  // TODO(bfredl): could be elided
-  api_free_dictionary(kv_A(args, 1).data.dictionary);
-  api_free_dictionary(kv_A(args, 2).data.dictionary);
 }
 
 static void remote_ui_highlight_set(UI *ui, int id)
@@ -772,11 +771,9 @@ static void remote_ui_highlight_set(UI *ui, int id)
     return;
   }
   data->hl_id = id;
-  Dictionary hl = hlattrs2dict(syn_attr2entry(id), ui->rgb);
-
-  ADD_C(args, DICTIONARY_OBJ(hl));
+  MAXSIZE_TEMP_DICT(dict, 16);
+  ADD_C(args, DICTIONARY_OBJ(hlattrs2dict(&dict, syn_attr2entry(id), ui->rgb)));
   push_call(ui, "highlight_set", args);
-  api_free_dictionary(kv_A(args, 0).data.dictionary);
 }
 
 /// "true" cursor used only for input focus
@@ -963,7 +960,7 @@ static Array translate_contents(UI *ui, Array contents)
     Array new_item = ARRAY_DICT_INIT;
     int attr = (int)item.items[0].data.integer;
     if (attr) {
-      Dictionary rgb_attrs = hlattrs2dict(syn_attr2entry(attr), ui->rgb);
+      Dictionary rgb_attrs = hlattrs2dict(NULL, syn_attr2entry(attr), ui->rgb);
       ADD(new_item, DICTIONARY_OBJ(rgb_attrs));
     } else {
       ADD(new_item, DICTIONARY_OBJ((Dictionary)ARRAY_DICT_INIT));
diff --git a/src/nvim/highlight.c b/src/nvim/highlight.c
index 0f20eb1905..83d819d3ec 100644
--- a/src/nvim/highlight.c
+++ b/src/nvim/highlight.c
@@ -719,93 +719,107 @@ Dictionary hl_get_attr_by_id(Integer attr_id, Boolean rgb, Error *err)
     return dic;
   }
 
-  return hlattrs2dict(syn_attr2entry((int)attr_id), rgb);
+  return hlattrs2dict(NULL, syn_attr2entry((int)attr_id), rgb);
 }
 
 /// Converts an HlAttrs into Dictionary
 ///
+/// @param[out] hl optional pre-allocated dictionary for return value
+///                if present, must be allocated with at least 16 elements!
 /// @param[in] aep data to convert
 /// @param use_rgb use 'gui*' settings if true, else resorts to 'cterm*'
-Dictionary hlattrs2dict(HlAttrs ae, bool use_rgb)
+Dictionary hlattrs2dict(Dictionary *hl_alloc, HlAttrs ae, bool use_rgb)
 {
-  Dictionary hl = ARRAY_DICT_INIT;
   int mask  = use_rgb ? ae.rgb_ae_attr : ae.cterm_ae_attr;
+  Dictionary hl = ARRAY_DICT_INIT;
+  if (hl_alloc) {
+    hl = *hl_alloc;
+  } else {
+    kv_ensure_space(hl, 16);
+  }
 
   if (mask & HL_BOLD) {
-    PUT(hl, "bold", BOOLEAN_OBJ(true));
+    PUT_C(hl, "bold", BOOLEAN_OBJ(true));
   }
 
   if (mask & HL_STANDOUT) {
-    PUT(hl, "standout", BOOLEAN_OBJ(true));
+    PUT_C(hl, "standout", BOOLEAN_OBJ(true));
   }
 
   if (mask & HL_UNDERLINE) {
-    PUT(hl, "underline", BOOLEAN_OBJ(true));
+    PUT_C(hl, "underline", BOOLEAN_OBJ(true));
   }
 
   if (mask & HL_UNDERCURL) {
-    PUT(hl, "undercurl", BOOLEAN_OBJ(true));
+    PUT_C(hl, "undercurl", BOOLEAN_OBJ(true));
   }
 
   if (mask & HL_UNDERDOUBLE) {
-    PUT(hl, "underdouble", BOOLEAN_OBJ(true));
+    PUT_C(hl, "underdouble", BOOLEAN_OBJ(true));
   }
 
   if (mask & HL_UNDERDOTTED) {
-    PUT(hl, "underdotted", BOOLEAN_OBJ(true));
+    PUT_C(hl, "underdotted", BOOLEAN_OBJ(true));
   }
 
   if (mask & HL_UNDERDASHED) {
-    PUT(hl, "underdashed", BOOLEAN_OBJ(true));
+    PUT_C(hl, "underdashed", BOOLEAN_OBJ(true));
   }
 
   if (mask & HL_ITALIC) {
-    PUT(hl, "italic", BOOLEAN_OBJ(true));
+    PUT_C(hl, "italic", BOOLEAN_OBJ(true));
   }
 
   if (mask & HL_INVERSE) {
-    PUT(hl, "reverse", BOOLEAN_OBJ(true));
+    PUT_C(hl, "reverse", BOOLEAN_OBJ(true));
   }
 
   if (mask & HL_STRIKETHROUGH) {
-    PUT(hl, "strikethrough", BOOLEAN_OBJ(true));
+    PUT_C(hl, "strikethrough", BOOLEAN_OBJ(true));
   }
 
   if (use_rgb) {
     if (mask & HL_FG_INDEXED) {
-      PUT(hl, "fg_indexed", BOOLEAN_OBJ(true));
+      PUT_C(hl, "fg_indexed", BOOLEAN_OBJ(true));
     }
 
     if (mask & HL_BG_INDEXED) {
-      PUT(hl, "bg_indexed", BOOLEAN_OBJ(true));
+      PUT_C(hl, "bg_indexed", BOOLEAN_OBJ(true));
     }
 
     if (ae.rgb_fg_color != -1) {
-      PUT(hl, "foreground", INTEGER_OBJ(ae.rgb_fg_color));
+      PUT_C(hl, "foreground", INTEGER_OBJ(ae.rgb_fg_color));
     }
 
     if (ae.rgb_bg_color != -1) {
-      PUT(hl, "background", INTEGER_OBJ(ae.rgb_bg_color));
+      PUT_C(hl, "background", INTEGER_OBJ(ae.rgb_bg_color));
     }
 
     if (ae.rgb_sp_color != -1) {
-      PUT(hl, "special", INTEGER_OBJ(ae.rgb_sp_color));
+      PUT_C(hl, "special", INTEGER_OBJ(ae.rgb_sp_color));
     }
   } else {
     if (ae.cterm_fg_color != 0) {
-      PUT(hl, "foreground", INTEGER_OBJ(ae.cterm_fg_color - 1));
+      PUT_C(hl, "foreground", INTEGER_OBJ(ae.cterm_fg_color - 1));
     }
 
     if (ae.cterm_bg_color != 0) {
-      PUT(hl, "background", INTEGER_OBJ(ae.cterm_bg_color - 1));
+      PUT_C(hl, "background", INTEGER_OBJ(ae.cterm_bg_color - 1));
     }
   }
 
   if (ae.hl_blend > -1) {
-    PUT(hl, "blend", INTEGER_OBJ(ae.hl_blend));
+    PUT_C(hl, "blend", INTEGER_OBJ(ae.hl_blend));
   }
 
-  return hl;
+  if (hl_alloc) {
+    *hl_alloc = hl;
+    return hl;
+  } else {
+    Dictionary allocated = copy_dictionary(hl);
+    kv_destroy(hl);
+    return allocated;
+  }
 }
 
 HlAttrs dict2hlattrs(Dict(highlight) *dict, bool use_rgb, int *link_id, Error *err)
diff --git a/src/nvim/highlight_group.c b/src/nvim/highlight_group.c
index d958b7b344..5027454222 100644
--- a/src/nvim/highlight_group.c
+++ b/src/nvim/highlight_group.c
@@ -1409,7 +1409,7 @@ Dictionary get_global_hl_defs(void)
     Dictionary attrs = ARRAY_DICT_INIT;
     HlGroup *h = &hl_table[i - 1];
     if (h->sg_attr > 0) {
-      attrs = hlattrs2dict(syn_attr2entry(h->sg_attr), true);
+      attrs = hlattrs2dict(NULL, syn_attr2entry(h->sg_attr), true);
     } else if (h->sg_link > 0) {
       const char *link = (const char *)hl_table[h->sg_link - 1].sg_name;
       PUT(attrs, "link", STRING_OBJ(cstr_to_string(link)));
diff --git a/src/nvim/main.c b/src/nvim/main.c
index b06b9630e2..c9403d9f19 100644
--- a/src/nvim/main.c
+++ b/src/nvim/main.c
@@ -202,7 +202,6 @@ void early_init(mparm_T *paramp)
   set_lang_var();               // set v:lang and v:ctype
 
   init_signs();
-  ui_comp_syn_init();
 }
 
 #ifdef MAKE_LIB
@@ -320,6 +319,7 @@ int main(int argc, char **argv)
   no_wait_return = true;
 
   init_highlight(true, false);  // Default highlight groups.
+  ui_comp_syn_init();
   TIME_MSG("init highlight");
 
   // Set the break level after the terminal is initialized.
diff --git a/test/functional/editor/put_spec.lua b/test/functional/editor/put_spec.lua
index cc9fce8f67..5050edff5c 100644
--- a/test/functional/editor/put_spec.lua
+++ b/test/functional/editor/put_spec.lua
@@ -879,9 +879,13 @@ describe('put command', function()
 	ine of words 2]], curbuf_contents())
     end)
 
-    local function bell_test(actions, should_ring)
-      local screen = Screen.new()
+    local screen
+    setup(function()
+      screen = Screen.new()
       screen:attach()
+    end)
+
+    local function bell_test(actions, should_ring)
       if should_ring then
         -- check bell is not set by nvim before the action
         screen:sleep(50)
@@ -899,7 +903,6 @@ describe('put command', function()
           end
         end
       end, unchanged=(not should_ring)}
-      screen:detach()
     end
 
     it('should not ring the bell with gp at end of line', function()
diff --git a/test/functional/ui/cursor_spec.lua b/test/functional/ui/cursor_spec.lua
index a1423c98a8..03cd4bfd06 100644
--- a/test/functional/ui/cursor_spec.lua
+++ b/test/functional/ui/cursor_spec.lua
@@ -215,7 +215,7 @@ describe('ui/cursor', function()
           m.hl_id = 60
           m.attr = {background = Screen.colors.DarkGray}
       end
-      if m.id_lm then m.id_lm = 65 end
+      if m.id_lm then m.id_lm = 61 end
     end
 
     -- Assert the new expectation.