perf(options): no-alloc keysets

scripts/linterrcodes.lua

@@ -33,7 +33,7 @@ local dup_allowed = {
   E317 = 4,
   E319 = 2,
   E423 = 3,
-  E474 = 59,
+  E474 = 60,
   E475 = 6,
   E482 = 3,
   E484 = 2,

src/gen/gen_options.lua

@@ -7,22 +7,41 @@ local options_meta = options.options
 local cstr = options.cstr
 local valid_scopes = options.valid_scopes
 local schema_values = options.schema_values
+-- Limit for a free-string (`str`) dict value; opt_strings_check() rejects anything longer.
+local keyset_str_max = 256
 
--- Object type of a `dict` key's value, for the generated KeySetLink table.
+-- Object type of a `dict` key's value, for the generated KeySetLink table. String-ish kinds
+-- (enum/str) are stored inline as fixed `char[]` (keyset_charsize); opt_fill() copies to them.
 local keyset_ctype = {
   num = 'kObjectTypeInteger',
   snum = 'kObjectTypeInteger',
   enum = 'kObjectTypeString',
   str = 'kObjectTypeString',
 }
--- C field type (an API type) for each `dict` key kind.
+-- C field type for `dict` key kinds (except enum/str, which use a fixed-size `char[]`).
 local keyset_ftype = {
   num = 'Integer',
   snum = 'Integer',
-  enum = 'String',
-  str = 'String',
 }
 
+--- Fixed `char[]` size for a string-ish dict key, to avoid allocations. An `enum` fits its longest
+--- value, a `str` is limited to `keyset_str_max`. Returns nil for non-string kinds (num/snum/flag).
+--- @param item vim.option_schema.dictkey
+--- @return integer?
+local function keyset_charsize(item)
+  local kind = type(item) == 'string' and 'flag' or item[2]
+  if kind == 'enum' then
+    local n = 0
+    for _, val in ipairs(item[3].values) do
+      n = math.max(n, #val)
+    end
+    return n + 1
+  elseif kind == 'str' then
+    return keyset_str_max
+  end
+  return nil
+end
+
 --- @param o vim.option_meta
 --- @return string
 local function get_values_var(o)
@@ -671,7 +690,12 @@ local function gen_keysets(output_file)
       for _, item in ipairs(o.schema.dict) do
         local key = type(item) == 'string' and item or item[1]
         local kind = type(item) == 'string' and 'flag' or item[2]
-        write(('  %s %s;'):format(keyset_ftype[kind] or 'Boolean', c_field_name(key)))
+        local charsize = keyset_charsize(item)
+        if charsize then
+          write(('  char %s[%d];'):format(c_field_name(key), charsize))
+        else
+          write(('  %s %s;'):format(keyset_ftype[kind] or 'Boolean', c_field_name(key)))
+        end
       end
       write(('} %s;'):format(kd))
       write('')
@@ -697,6 +721,14 @@ local function gen_keysets(output_file)
     end
   end
 
+  -- Union of all keyset types, for `opt_keyset(NULL, …)`.
+  write('')
+  write('typedef union {')
+  for _, s in ipairs(struct_opts) do
+    write(('  %s %s;'):format(s.kd, s.abbr))
+  end
+  write('} OptKeyDict;')
+
   -- Dispatch from option index to its keyset handle (opt_dict_info); NULL for non-dict options.
   write('')
   write('static inline const OptDictInfo *opt_dict_info(OptIndex opt_idx)')

src/nvim/diff.c

@@ -2659,7 +2659,7 @@ int diffanchors_changed(bool buflocal)
 /// @return  FAIL only for the cross-part "horizontal" + "vertical" conflict.
 int diffopt_changed(void)
 {
-  OptKeyDict_dip *v = opt_keyset_alloc(kOptDiffopt, p_dip);
+  OptKeyDict_dip *v = opt_keyset(p_dip, kOptDiffopt, NULL);
 
   int flags = (v->filler ? DIFF_FILLER : 0) | (v->anchor ? DIFF_ANCHOR : 0)
               | (v->iblank ? DIFF_IBLANK : 0) | (v->icase ? DIFF_ICASE : 0)
@@ -2673,11 +2673,11 @@ int diffopt_changed(void)
   }
   if (HAS_KEY(v, dip, inline_)) {
     flags &= ~ALL_INLINE;
-    if (option_slice_eq(v->inline_.data, v->inline_.size, "simple")) {
+    if (strequal(v->inline_, "simple")) {
       flags |= DIFF_INLINE_SIMPLE;
-    } else if (option_slice_eq(v->inline_.data, v->inline_.size, "char")) {
+    } else if (strequal(v->inline_, "char")) {
       flags |= DIFF_INLINE_CHAR;
-    } else if (option_slice_eq(v->inline_.data, v->inline_.size, "word")) {
+    } else if (strequal(v->inline_, "word")) {
       flags |= DIFF_INLINE_WORD;
     } else {
       flags |= DIFF_INLINE_NONE;
@@ -2685,11 +2685,11 @@ int diffopt_changed(void)
   }
   int algorithm = v->indent_heuristic ? XDF_INDENT_HEURISTIC : 0;
   if (HAS_KEY(v, dip, algorithm)) {
-    if (option_slice_eq(v->algorithm.data, v->algorithm.size, "minimal")) {
+    if (strequal(v->algorithm, "minimal")) {
       algorithm |= XDF_NEED_MINIMAL;
-    } else if (option_slice_eq(v->algorithm.data, v->algorithm.size, "patience")) {
+    } else if (strequal(v->algorithm, "patience")) {
       algorithm |= XDF_PATIENCE_DIFF;
-    } else if (option_slice_eq(v->algorithm.data, v->algorithm.size, "histogram")) {
+    } else if (strequal(v->algorithm, "histogram")) {
       algorithm |= XDF_HISTOGRAM_DIFF;
     }  // else "myers" -> 0
   }
@@ -2720,7 +2720,6 @@ int diffopt_changed(void)
     check_scrollbind(0, 0);
   }
 
-  opt_keyset_free(kOptDiffopt, v);
   return ret;
 }
 

src/nvim/indent.c

@@ -37,6 +37,7 @@
 #include "nvim/option.h"
 #include "nvim/option_defs.h"
 #include "nvim/option_vars.h"
+#include "nvim/optionstr.h"
 #include "nvim/os/input.h"
 #include "nvim/plines.h"
 #include "nvim/pos_defs.h"
@@ -789,13 +790,12 @@ void briopt_check(win_T *wp)
   }
   // 'breakindentopt' is stored as its ":set" string (validated when set); reify it into a keyset and
   // map that onto the applied per-window fields.
-  OptKeyDict_briopt *v = opt_keyset_alloc(kOptBreakindentopt, wp->w_p_briopt);
+  OptKeyDict_briopt *v = opt_keyset(wp->w_p_briopt, kOptBreakindentopt, NULL);
   wp->w_briopt_shift = HAS_KEY(v, briopt, shift) ? (int)v->shift : 0;
   wp->w_briopt_min = HAS_KEY(v, briopt, min) ? (int)v->min : 20;
   wp->w_briopt_sbr = HAS_KEY(v, briopt, sbr);
   wp->w_briopt_list = HAS_KEY(v, briopt, list) ? (int)v->list : 0;
   wp->w_briopt_vcol = HAS_KEY(v, briopt, column) ? (int)v->column : 0;
-  opt_keyset_free(kOptBreakindentopt, v);
 }
 
 // Return appropriate space number for breakindent, taking influencing

src/nvim/option.c

@@ -3337,38 +3337,12 @@ OptIndex find_option(const char *const name)
 }
 
 /// True if `opt_idx` is a dict option (`schema.dict` in options.lua, e.g. 'diffopt'). Stored as its
-/// canonical ":set" string; reified on-demand (`opt_keyset_alloc()`) for validation and applying.
+/// canonical ":set" string; reified on-demand (`opt_keyset()`) for validation and applying.
 bool is_dict_option(OptIndex opt_idx)
 {
   return opt_dict_info(opt_idx) != NULL;
 }
 
-/// Parse a dict option's ":set" string to an allocated keyset (`OptKeyDict_…`). A NULL string
-/// yields an all-unset keyset. Free it with `opt_keyset_free()`.
-void *opt_keyset_alloc(OptIndex opt_idx, const char *str)
-{
-  const OptDictInfo *si = opt_dict_info(opt_idx);
-  void *keyset = xcalloc(1, si->size);
-  if (str != NULL) {
-    opt_fill(str, si->get_field, keyset);
-  }
-  return keyset;
-}
-
-/// Free a keyset from `opt_keyset_alloc()`: its owned `String` fields, then the keyset itself.
-void opt_keyset_free(OptIndex opt_idx, void *keyset)
-{
-  if (keyset == NULL) {
-    return;
-  }
-  for (const KeySetLink *f = opt_dict_info(opt_idx)->table; f->str != NULL; f++) {
-    if (f->type == kObjectTypeString) {
-      api_free_string(*(String *)((char *)keyset + f->ptr_off));
-    }
-  }
-  xfree(keyset);
-}
-
 /// Free an allocated OptVal.
 void optval_free(OptVal o)
 {

src/nvim/optionstr.c

@@ -1560,14 +1560,13 @@ int expand_set_mouse(optexpand_T *args, int *numMatches, char ***matches)
 /// @return error message, NULL if it's OK.
 const char *did_set_mousescroll(optset_T *args FUNC_ATTR_UNUSED)
 {
-  OptKeyDict_mousescroll *v = opt_keyset_alloc(kOptMousescroll, p_mousescroll);
+  OptKeyDict_mousescroll *v = opt_keyset(p_mousescroll, kOptMousescroll, NULL);
   // An empty value sets no direction; reject it (mousescroll always needs at least one).
   bool has_dir = HAS_KEY(v, mousescroll, hor) || HAS_KEY(v, mousescroll, ver);
   if (has_dir) {
     p_mousescroll_hor = HAS_KEY(v, mousescroll, hor) ? (int)v->hor : MOUSESCROLL_HOR_DFLT;
     p_mousescroll_vert = HAS_KEY(v, mousescroll, ver) ? (int)v->ver : MOUSESCROLL_VERT_DFLT;
   }
-  opt_keyset_free(kOptMousescroll, v);
   return has_dir ? NULL : e_invarg;
 }
 
@@ -2295,15 +2294,21 @@ const char *opt_strings_check(const char *val, const OptSchemaItem *schema, char
       if (v == NULL) {
         return opt_schema_err(errbuf, errbuflen, N_("E474: '%s' requires a value"), key, keylen);
       }
+      if (vlen >= 256) {  // This limit must match `keyset_str_max` (gen_options.lua).
+        return opt_schema_err(errbuf, errbuflen, N_("E474: '%s' value is too long"), key, keylen);
+      }
       break;
     }
   }
   return NULL;
 }
 
-/// Parses a validated dict option ":set" string into its keyset (`OptKeyDict_…`), see also
-/// `api_dict_to_keydict()`. The keyset owns its `String` fields, so free it with `opt_keyset_free()`
-/// (which frees those fields), never a bare `xfree()`. Callers use `opt_keyset_alloc()`.
+/// Parses a validated dict option into `out`. String-ish values live in fixed `char[]` fields (not
+/// allocated), so the keyset owns nothing and needs no free.
+///
+/// @param value Option value in ":set"-string form.
+/// @param get_field Perfect-hash fn.
+/// @param out Caller-provided keyset storage (`OptKeyDict_…`).
 void opt_fill(const char *value, FieldHashfn get_field, void *out)
   FUNC_ATTR_NONNULL_ALL
 {
@@ -2325,9 +2330,9 @@ void opt_fill(const char *value, FieldHashfn get_field, void *out)
       break;
     }
     case kObjectTypeString:
-      // A repeated key is "last wins" (e.g. 'diffopt' "inline:char,inline:word"); free the old one.
-      xfree(((String *)field)->data);
-      *(String *)field = (String){ .data = xmemdupz(v, vlen), .size = vlen };  // owned by the keyset
+      // A repeated key is "last wins" (overwritten). opt_strings_check() bounds the size.
+      memcpy(field, v, vlen);
+      ((char *)field)[vlen] = NUL;
       break;
     default:
       break;
@@ -2336,6 +2341,28 @@ void opt_fill(const char *value, FieldHashfn get_field, void *out)
   }
 }
 
+/// Reify a dict-option value to a keyset (`OptKeyDict_…`). String values live in fixed char[]
+/// fields (not allocated), so the keyset owns nothing and needs no free.
+///
+/// @param value   Option value in ":set"-string form.
+/// @param opt_idx Option idx.
+/// @param keyset  Caller-provided storage, or NULL to use shared static storage (only for callers
+///                that never "overlap"). Cleared first; a NUL string yields an all-unset keyset.
+/// @return  the keyset (for convenient assignment).
+void *opt_keyset(const char *value, OptIndex opt_idx, void *keyset)
+{
+  static OptKeyDict shared;
+  if (keyset == NULL) {
+    keyset = &shared;
+  }
+  const OptDictInfo *si = opt_dict_info(opt_idx);
+  memset(keyset, 0, si->size);
+  if (value != NULL) {
+    opt_fill(value, si->get_field, keyset);
+  }
+  return keyset;
+}
+
 /// @return  OK if "p" is a valid fileformat name, FAIL otherwise.
 int check_ff_value(char *p)
 {

src/nvim/winfloat.c

@@ -585,7 +585,7 @@ bool win_previewpopup_config(char *value, WinConfig *config)
   FUNC_ATTR_NONNULL_ALL
 {
   // Basic validation was done by opt_strings_check; do more validation here.
-  OptKeyDict_pvp *v = opt_keyset_alloc(kOptPreviewpopup, value);
+  OptKeyDict_pvp *v = opt_keyset(value, kOptPreviewpopup, NULL);
   bool ok = true;
 
   if ((HAS_KEY(v, pvp, height) && v->height < 1) || (HAS_KEY(v, pvp, width) && v->width < 1)) {
@@ -595,15 +595,13 @@ bool win_previewpopup_config(char *value, WinConfig *config)
   bool has_border = HAS_KEY(v, pvp, border);
   if (ok && has_border) {
     Error err = ERROR_INIT;
-    ok = parse_winborder(config, v->border.data, &err);
+    ok = parse_winborder(config, v->border, &err);
     api_clear_error(&err);
   }
 
   config->height = HAS_KEY(v, pvp, height) ? (int)v->height : 0;
   config->width = HAS_KEY(v, pvp, width) ? (int)v->width : 0;
 
-  opt_keyset_free(kOptPreviewpopup, v);
-
   if (!has_border) {
     config->border = false;
     if (*p_winborder != NUL) {