perf(events): store autocommands in flat vectors (#23256)

Instead of nested linked lists, store autocommands in a flat, contiguous
kvec_t, with one kvec_t per event type. Previously patterns were stored
in each node of the outer linked list, so they can be matched only once
on repeating patterns. They are now reference counted and referenced in
each autocommand, and matching is skipped if the pattern repeats. Speeds
up creation and deletion, execution is not affected.

Co-authored-by: ii14 <ii14@users.noreply.github.com>

src/nvim/api/autocmd.c

@@ -225,8 +225,12 @@ Array nvim_get_autocmds(Dict(get_autocmds) *opts, Error *err)
       continue;
     }
 
-    for (AutoPat *ap = au_get_autopat_for_event(event); ap != NULL; ap = ap->next) {
+    AutoCmdVec *acs = au_get_autocmds_for_event(event);
-      if (ap->cmds == NULL) {
+    for (size_t i = 0; i < kv_size(*acs); i++) {
+      AutoCmd *const ac = &kv_A(*acs, i);
+      AutoPat *const ap = ac->pat;
+
+      if (ap == NULL) {
         continue;
       }
 
@@ -238,19 +242,16 @@ Array nvim_get_autocmds(Dict(get_autocmds) *opts, Error *err)
       // Skip 'pattern' from invalid patterns if passed.
       if (pattern_filter_count > 0) {
         bool passed = false;
-        for (int i = 0; i < pattern_filter_count; i++) {
+        for (int j = 0; j < pattern_filter_count; j++) {
-          assert(i < AUCMD_MAX_PATTERNS);
+          assert(j < AUCMD_MAX_PATTERNS);
-          assert(pattern_filters[i]);
+          assert(pattern_filters[j]);
 
-          char *pat = pattern_filters[i];
+          char *pat = pattern_filters[j];
           int patlen = (int)strlen(pat);
 
           if (aupat_is_buflocal(pat, patlen)) {
-            aupat_normalize_buflocal_pat(pattern_buflocal,
+            aupat_normalize_buflocal_pat(pattern_buflocal, pat, patlen,
-                                         pat,
-                                         patlen,
                                          aupat_get_buflocal_nr(pat, patlen));
-
             pat = pattern_buflocal;
           }
 
@@ -265,85 +266,71 @@ Array nvim_get_autocmds(Dict(get_autocmds) *opts, Error *err)
         }
       }
 
-      for (AutoCmd *ac = ap->cmds; ac != NULL; ac = ac->next) {
+      Dictionary autocmd_info = ARRAY_DICT_INIT;
-        if (aucmd_exec_is_deleted(ac->exec)) {
-          continue;
-        }
 
-        Dictionary autocmd_info = ARRAY_DICT_INIT;
+      if (ap->group != AUGROUP_DEFAULT) {
-
+        PUT(autocmd_info, "group", INTEGER_OBJ(ap->group));
-        if (ap->group != AUGROUP_DEFAULT) {
+        PUT(autocmd_info, "group_name", CSTR_TO_OBJ(augroup_name(ap->group)));
-          PUT(autocmd_info, "group", INTEGER_OBJ(ap->group));
-          PUT(autocmd_info, "group_name", CSTR_TO_OBJ(augroup_name(ap->group)));
-        }
-
-        if (ac->id > 0) {
-          PUT(autocmd_info, "id", INTEGER_OBJ(ac->id));
-        }
-
-        if (ac->desc != NULL) {
-          PUT(autocmd_info, "desc", CSTR_TO_OBJ(ac->desc));
-        }
-
-        if (ac->exec.type == CALLABLE_CB) {
-          PUT(autocmd_info, "command", STRING_OBJ(STRING_INIT));
-
-          Callback *cb = &ac->exec.callable.cb;
-          switch (cb->type) {
-          case kCallbackLua:
-            if (nlua_ref_is_function(cb->data.luaref)) {
-              PUT(autocmd_info, "callback", LUAREF_OBJ(api_new_luaref(cb->data.luaref)));
-            }
-            break;
-          case kCallbackFuncref:
-          case kCallbackPartial:
-            PUT(autocmd_info, "callback", STRING_OBJ(cstr_as_string(callback_to_string(cb))));
-            break;
-          default:
-            abort();
-          }
-        } else {
-          PUT(autocmd_info,
-              "command",
-              STRING_OBJ(cstr_as_string(xstrdup(ac->exec.callable.cmd))));
-        }
-
-        PUT(autocmd_info,
-            "pattern",
-            STRING_OBJ(cstr_to_string(ap->pat)));
-
-        PUT(autocmd_info,
-            "event",
-            STRING_OBJ(cstr_to_string((char *)event_nr2name(event))));
-
-        PUT(autocmd_info, "once", BOOLEAN_OBJ(ac->once));
-
-        if (ap->buflocal_nr) {
-          PUT(autocmd_info, "buflocal", BOOLEAN_OBJ(true));
-          PUT(autocmd_info, "buffer", INTEGER_OBJ(ap->buflocal_nr));
-        } else {
-          PUT(autocmd_info, "buflocal", BOOLEAN_OBJ(false));
-        }
-
-        // TODO(sctx): It would be good to unify script_ctx to actually work with lua
-        //  right now it's just super weird, and never really gives you the info that
-        //  you would expect from this.
-        //
-        //  I think we should be able to get the line number, filename, etc. from lua
-        //  when we're executing something, and it should be easy to then save that
-        //  info here.
-        //
-        //  I think it's a big loss not getting line numbers of where options, autocmds,
-        //  etc. are set (just getting "Sourced (lua)" or something is not that helpful.
-        //
-        //  Once we do that, we can put these into the autocmd_info, but I don't think it's
-        //  useful to do that at this time.
-        //
-        // PUT(autocmd_info, "sid", INTEGER_OBJ(ac->script_ctx.sc_sid));
-        // PUT(autocmd_info, "lnum", INTEGER_OBJ(ac->script_ctx.sc_lnum));
-
-        ADD(autocmd_list, DICTIONARY_OBJ(autocmd_info));
       }
+
+      if (ac->id > 0) {
+        PUT(autocmd_info, "id", INTEGER_OBJ(ac->id));
+      }
+
+      if (ac->desc != NULL) {
+        PUT(autocmd_info, "desc", CSTR_TO_OBJ(ac->desc));
+      }
+
+      if (ac->exec.type == CALLABLE_CB) {
+        PUT(autocmd_info, "command", STRING_OBJ(STRING_INIT));
+
+        Callback *cb = &ac->exec.callable.cb;
+        switch (cb->type) {
+        case kCallbackLua:
+          if (nlua_ref_is_function(cb->data.luaref)) {
+            PUT(autocmd_info, "callback", LUAREF_OBJ(api_new_luaref(cb->data.luaref)));
+          }
+          break;
+        case kCallbackFuncref:
+        case kCallbackPartial:
+          PUT(autocmd_info, "callback", STRING_OBJ(cstr_as_string(callback_to_string(cb))));
+          break;
+        default:
+          abort();
+        }
+      } else {
+        PUT(autocmd_info, "command", STRING_OBJ(cstr_as_string(xstrdup(ac->exec.callable.cmd))));
+      }
+
+      PUT(autocmd_info, "pattern", STRING_OBJ(cstr_to_string(ap->pat)));
+      PUT(autocmd_info, "event", STRING_OBJ(cstr_to_string(event_nr2name(event))));
+      PUT(autocmd_info, "once", BOOLEAN_OBJ(ac->once));
+
+      if (ap->buflocal_nr) {
+        PUT(autocmd_info, "buflocal", BOOLEAN_OBJ(true));
+        PUT(autocmd_info, "buffer", INTEGER_OBJ(ap->buflocal_nr));
+      } else {
+        PUT(autocmd_info, "buflocal", BOOLEAN_OBJ(false));
+      }
+
+      // TODO(sctx): It would be good to unify script_ctx to actually work with lua
+      //  right now it's just super weird, and never really gives you the info that
+      //  you would expect from this.
+      //
+      //  I think we should be able to get the line number, filename, etc. from lua
+      //  when we're executing something, and it should be easy to then save that
+      //  info here.
+      //
+      //  I think it's a big loss not getting line numbers of where options, autocmds,
+      //  etc. are set (just getting "Sourced (lua)" or something is not that helpful.
+      //
+      //  Once we do that, we can put these into the autocmd_info, but I don't think it's
+      //  useful to do that at this time.
+      //
+      // PUT(autocmd_info, "sid", INTEGER_OBJ(ac->script_ctx.sc_sid));
+      // PUT(autocmd_info, "lnum", INTEGER_OBJ(ac->script_ctx.sc_lnum));
+
+      ADD(autocmd_list, DICTIONARY_OBJ(autocmd_info));
     }
   }
 
@@ -663,7 +650,7 @@ Integer nvim_create_augroup(uint64_t channel_id, String name, Dict(create_augrou
 
     if (clear_autocmds) {
       FOR_ALL_AUEVENTS(event) {
-        aupat_del_for_event_and_group(event, augroup);
+        aucmd_del_for_event_and_group(event, augroup);
       }
     }
   });
@@ -866,7 +853,7 @@ static bool get_patterns_from_pattern_or_buf(Array *patterns, Object pattern, Ob
     Object *v = &pattern;
 
     if (v->type == kObjectTypeString) {
-      char *pat = v->data.string.data;
+      const char *pat = v->data.string.data;
       size_t patlen = aucmd_pattern_length(pat);
       while (patlen) {
         ADD(*patterns, STRING_OBJ(cbuf_to_string(pat, patlen)));
@@ -881,7 +868,7 @@ static bool get_patterns_from_pattern_or_buf(Array *patterns, Object pattern, Ob
 
       Array array = v->data.array;
       FOREACH_ITEM(array, entry, {
-        char *pat = entry.data.string.data;
+        const char *pat = entry.data.string.data;
         size_t patlen = aucmd_pattern_length(pat);
         while (patlen) {
           ADD(*patterns, STRING_OBJ(cbuf_to_string(pat, patlen)));

src/nvim/autocmd.h

@@ -12,9 +12,7 @@
 #include "nvim/regexp_defs.h"
 #include "nvim/types.h"
 
-struct AutoCmd_S;
 struct AutoPatCmd_S;
-struct AutoPat_S;
 
 // event_T definition
 #ifdef INCLUDE_GENERATED_DECLARATIONS
@@ -35,49 +33,45 @@ typedef struct {
   int save_State;                 ///< saved State
 } aco_save_T;
 
-typedef struct AutoCmd_S AutoCmd;
+typedef struct {
-struct AutoCmd_S {
+  size_t refcount;          ///< Reference count (freed when reaches zero)
-  AucmdExecutable exec;
+  char *pat;                ///< Pattern as typed
-  bool once;                            // "One shot": removed after execution
+  regprog_T *reg_prog;      ///< Compiled regprog for pattern
-  bool nested;                          // If autocommands nest here
+  int group;                ///< Group ID
-  bool last;                            // last command in list
+  int patlen;               ///< strlen() of pat
-  int64_t id;                           // ID used for uniquely tracking an autocmd.
+  int buflocal_nr;          ///< !=0 for buffer-local AutoPat
-  sctx_T script_ctx;                    // script context where it is defined
+  char allow_dirs;          ///< Pattern may match whole path
-  char *desc;                           // Description for the autocmd.
+} AutoPat;
-  AutoCmd *next;                        // Next AutoCmd in list
-};
 
-typedef struct AutoPat_S AutoPat;
+typedef struct {
-struct AutoPat_S {
+  AucmdExecutable exec;     ///< Command or callback function
-  AutoPat *next;                        // next AutoPat in AutoPat list; MUST
+  AutoPat *pat;             ///< Pattern reference (NULL when autocmd was removed)
-                                        // be the first entry
+  int64_t id;               ///< ID used for uniquely tracking an autocmd
-  char *pat;                            // pattern as typed (NULL when pattern
+  char *desc;               ///< Description for the autocmd
-                                        // has been removed)
+  sctx_T script_ctx;        ///< Script context where it is defined
-  regprog_T *reg_prog;                  // compiled regprog for pattern
+  bool once;                ///< "One shot": removed after execution
-  AutoCmd *cmds;                        // list of commands to do
+  bool nested;              ///< If autocommands nest here
-  int group;                            // group ID
+} AutoCmd;
-  int patlen;                           // strlen() of pat
-  int buflocal_nr;                      // !=0 for buffer-local AutoPat
-  char allow_dirs;                      // Pattern may match whole path
-  char last;                            // last pattern for apply_autocmds()
-};
 
 /// Struct used to keep status while executing autocommands for an event.
 typedef struct AutoPatCmd_S AutoPatCmd;
 struct AutoPatCmd_S {
-  AutoPat *curpat;          // next AutoPat to examine
+  AutoPat *lastpat;         ///< Last matched AutoPat
-  AutoCmd *nextcmd;         // next AutoCmd to execute
+  size_t auidx;             ///< Current autocmd index to execute
-  int group;                // group being used
+  size_t ausize;            ///< Saved AutoCmd vector size
-  char *fname;              // fname to match with
+  char *fname;              ///< Fname to match with
-  char *sfname;             // sfname to match with
+  char *sfname;             ///< Sfname to match with
-  char *tail;               // tail of fname
+  char *tail;               ///< Tail of fname
-  event_T event;            // current event
+  int group;                ///< Group being used
-  sctx_T script_ctx;        // script context where it is defined
+  event_T event;            ///< Current event
-  int arg_bufnr;            // initially equal to <abuf>, set to zero when buf is deleted
+  sctx_T script_ctx;        ///< Script context where it is defined
-  Object *data;             // arbitrary data
+  int arg_bufnr;            ///< Initially equal to <abuf>, set to zero when buf is deleted
-  AutoPatCmd *next;         // chain of active apc-s for auto-invalidation
+  Object *data;             ///< Arbitrary data
+  AutoPatCmd *next;         ///< Chain of active apc-s for auto-invalidation
 };
 
+typedef kvec_t(AutoCmd) AutoCmdVec;
+
 // Set by the apply_autocmds_group function if the given event is equal to
 // EVENT_FILETYPE. Used by the readfile function in order to determine if
 // EVENT_BUFREADPOST triggered the EVENT_FILETYPE.

src/nvim/ex_docmd.c

@@ -1885,8 +1885,7 @@ static char *do_one_cmd(char **cmdlinep, int flags, cstack_T *cstack, LineGetter
       // avoid that a function call in 'statusline' does this
       && !getline_equal(fgetline, cookie, get_func_line)
       // avoid that an autocommand, e.g. QuitPre, does this
-      && !getline_equal(fgetline, cookie,
+      && !getline_equal(fgetline, cookie, getnextac)) {
-                        getnextac)) {
     quitmore--;
   }
 

src/nvim/generators/gen_events.lua

@@ -35,27 +35,24 @@ names_tgt:write('\n  {0, NULL, (event_T)0},')
 enum_tgt:write('\n} event_T;\n')
 names_tgt:write('\n};\n')
 
-local gen_autopat_events = function(name)
+do
-  names_tgt:write(string.format('\nstatic AutoPat *%s[NUM_EVENTS] = {\n ', name))
+  names_tgt:write('\nstatic AutoCmdVec autocmds[NUM_EVENTS] = {\n ')
   local line_len = 1
   for _ = 1,((#events) - 1) do
-    line_len = line_len + #(' NULL,')
+    line_len = line_len + #(' KV_INITIAL_VALUE,')
     if line_len > 80 then
       names_tgt:write('\n ')
-      line_len = 1 + #(' NULL,')
+      line_len = 1 + #(' KV_INITIAL_VALUE,')
     end
-    names_tgt:write(' NULL,')
+    names_tgt:write(' KV_INITIAL_VALUE,')
   end
-  if line_len + #(' NULL') > 80 then
+  if line_len + #(' KV_INITIAL_VALUE') > 80 then
-    names_tgt:write('\n  NULL')
+    names_tgt:write('\n  KV_INITIAL_VALUE')
   else
-    names_tgt:write(' NULL')
+    names_tgt:write(' KV_INITIAL_VALUE')
   end
   names_tgt:write('\n};\n')
 end
 
-gen_autopat_events("first_autopat")
-gen_autopat_events("last_autopat")
-
 enum_tgt:close()
 names_tgt:close()

test/benchmark/autocmd_spec.lua

@@ -0,0 +1,175 @@
+local helpers = require('test.functional.helpers')(after_each)
+
+local clear = helpers.clear
+local exec_lua = helpers.exec_lua
+
+local N = 7500
+
+describe('autocmd perf', function()
+  before_each(function()
+    clear()
+
+    exec_lua([[
+      out = {}
+      function start()
+        ts = vim.loop.hrtime()
+      end
+      function stop(name)
+        out[#out+1] = ('%14.6f ms - %s'):format((vim.loop.hrtime() - ts) / 1000000, name)
+      end
+    ]])
+  end)
+
+  after_each(function()
+    for _, line in ipairs(exec_lua([[return out]])) do
+      print(line)
+    end
+  end)
+
+  it('nvim_create_autocmd, nvim_del_autocmd (same pattern)', function()
+    exec_lua([[
+      local N = ...
+      local ids = {}
+
+      start()
+        for i = 1, N do
+          ids[i] = vim.api.nvim_create_autocmd('User', {
+            pattern = 'Benchmark',
+            command = 'eval 0', -- noop
+          })
+        end
+      stop('nvim_create_autocmd')
+
+      start()
+        for i = 1, N do
+          vim.api.nvim_del_autocmd(ids[i])
+        end
+      stop('nvim_del_autocmd')
+    ]], N)
+  end)
+
+  it('nvim_create_autocmd, nvim_del_autocmd (unique patterns)', function()
+    exec_lua([[
+      local N = ...
+      local ids = {}
+
+      start()
+        for i = 1, N do
+          ids[i] = vim.api.nvim_create_autocmd('User', {
+            pattern = 'Benchmark' .. i,
+            command = 'eval 0', -- noop
+          })
+        end
+      stop('nvim_create_autocmd')
+
+      start()
+        for i = 1, N do
+          vim.api.nvim_del_autocmd(ids[i])
+        end
+      stop('nvim_del_autocmd')
+    ]], N)
+  end)
+
+  it('nvim_create_autocmd + nvim_del_autocmd', function()
+    exec_lua([[
+      local N = ...
+
+      start()
+        for _ = 1, N do
+          local id = vim.api.nvim_create_autocmd('User', {
+            pattern = 'Benchmark',
+            command = 'eval 0', -- noop
+          })
+          vim.api.nvim_del_autocmd(id)
+        end
+      stop('nvim_create_autocmd + nvim_del_autocmd')
+    ]], N)
+  end)
+
+  it('nvim_exec_autocmds (same pattern)', function()
+    exec_lua([[
+      local N = ...
+
+      for i = 1, N do
+        vim.api.nvim_create_autocmd('User', {
+          pattern = 'Benchmark',
+          command = 'eval 0', -- noop
+        })
+      end
+
+      start()
+        vim.api.nvim_exec_autocmds('User', { pattern = 'Benchmark', modeline = false })
+      stop('nvim_exec_autocmds')
+    ]], N)
+  end)
+
+  it('nvim_del_augroup_by_id', function()
+    exec_lua([[
+      local N = ...
+      local group = vim.api.nvim_create_augroup('Benchmark', {})
+
+      for i = 1, N do
+        vim.api.nvim_create_autocmd('User', {
+          pattern = 'Benchmark',
+          command = 'eval 0', -- noop
+          group = group,
+        })
+      end
+
+      start()
+        vim.api.nvim_del_augroup_by_id(group)
+      stop('nvim_del_augroup_by_id')
+    ]], N)
+  end)
+
+  it('nvim_del_augroup_by_name', function()
+    exec_lua([[
+      local N = ...
+      local group = vim.api.nvim_create_augroup('Benchmark', {})
+
+      for i = 1, N do
+        vim.api.nvim_create_autocmd('User', {
+          pattern = 'Benchmark',
+          command = 'eval 0', -- noop
+          group = group,
+        })
+      end
+
+      start()
+        vim.api.nvim_del_augroup_by_name('Benchmark')
+      stop('nvim_del_augroup_by_id')
+    ]], N)
+  end)
+
+  it(':autocmd, :autocmd! (same pattern)', function()
+    exec_lua([[
+      local N = ...
+
+      start()
+        for i = 1, N do
+          vim.cmd('autocmd User Benchmark eval 0')
+        end
+      stop(':autocmd')
+
+      start()
+        vim.cmd('autocmd! User Benchmark')
+      stop(':autocmd!')
+    ]], N)
+  end)
+
+  it(':autocmd, :autocmd! (unique patterns)', function()
+    exec_lua([[
+      local N = ...
+
+      start()
+        for i = 1, N do
+          vim.cmd(('autocmd User Benchmark%d eval 0'):format(i))
+        end
+      stop(':autocmd')
+
+      start()
+        vim.cmd('autocmd! User')
+      stop(':autocmd!')
+    ]], N)
+  end)
+end)

test/functional/autocmd/autocmd_spec.lua

@@ -611,4 +611,22 @@ describe('autocmd', function()
       eq(4, #meths.get_autocmds { event = "BufReadCmd", group = "TestingPatterns" })
     end)
   end)
+
+  it('no use-after-free when adding autocommands from a callback', function()
+    exec_lua [[
+      vim.cmd "autocmd! TabNew"
+      vim.g.count = 0
+      vim.api.nvim_create_autocmd('TabNew', {
+        callback = function()
+          vim.g.count = vim.g.count + 1
+          for _ = 1, 100 do
+            vim.cmd "autocmd TabNew * let g:count += 1"
+          end
+          return true
+        end,
+      })
+      vim.cmd "tabnew"
+    ]]
+    eq(1, eval('g:count'))  -- Added autocommands should not be executed
+  end)
 end)

test/functional/autocmd/show_spec.lua

@@ -180,4 +180,45 @@ describe(":autocmd", function()
       test_3  User
           B         echo "B3"]]), funcs.execute('autocmd test_3 * B'))
   end)
+
+  it('should skip consecutive patterns', function()
+    exec([[
+      autocmd! BufEnter
+      augroup test_1
+        autocmd BufEnter A echo 'A'
+        autocmd BufEnter A echo 'B'
+        autocmd BufEnter A echo 'C'
+        autocmd BufEnter B echo 'D'
+        autocmd BufEnter B echo 'E'
+        autocmd BufEnter B echo 'F'
+      augroup END
+      augroup test_2
+        autocmd BufEnter C echo 'A'
+        autocmd BufEnter C echo 'B'
+        autocmd BufEnter C echo 'C'
+        autocmd BufEnter D echo 'D'
+        autocmd BufEnter D echo 'E'
+        autocmd BufEnter D echo 'F'
+      augroup END
+
+      let g:output = execute('autocmd BufEnter')
+    ]])
+    eq(dedent([[
+
+      --- Autocommands ---
+      test_1  BufEnter
+          A         echo 'A'
+                    echo 'B'
+                    echo 'C'
+          B         echo 'D'
+                    echo 'E'
+                    echo 'F'
+      test_2  BufEnter
+          C         echo 'A'
+                    echo 'B'
+                    echo 'C'
+          D         echo 'D'
+                    echo 'E'
+                    echo 'F']]), eval('g:output'))
+  end)
 end)