vim-patch:9.2.0909: insert completion is slow to collect many matches (#41167)

Problem:  ins_compl_add() checks for a duplicate by scanning the whole
          match list, making collection of N matches quadratic.
Solution: Look matches up in a hashtab instead; each entry counts the
          matches with that string (Samuel Schlesinger).

closes: vim/vim#20926

31b7b1a7da

Co-authored-by: Samuel Schlesinger <sgschlesinger@gmail.com>
Co-authored-by: Claude <noreply@anthropic.com>

src/nvim/insexpand.c

@@ -227,6 +227,44 @@ static compl_T *compl_shown_match = NULL;
 static compl_T *compl_old_match = NULL;
 static compl_T *compl_preselect_match = NULL;
 
+/// Hashtab with the strings of the matches in the list above, except the
+/// original-text entries.  Used to make the duplicate check O(1) instead of
+/// a scan of the whole list.  Each entry owns a copy of the string and
+/// counts the matches with that string, so that when matches were added
+/// with "adup" the entry remains until the last match with the string is
+/// removed.
+typedef struct {
+  int cse_count;   // number of matches with this string
+  char cse_str[];  // the string
+} complstr_T;
+
+#define CSE_OFF         (offsetof(complstr_T, cse_str))
+#define HI2CSE(hi)      ((complstr_T *)((hi)->hi_key - CSE_OFF))
+
+static hashtab_T compl_strings_ht;
+
+/// Count the string of a new match in the duplicate-check hashtab.
+/// "hash" is the hash of "str" when it is not zero, saving hashing the
+/// string again.
+static void compl_strings_add(const char *str, size_t len, hash_T hash)
+{
+  if (compl_strings_ht.ht_array == NULL) {
+    hash_init(&compl_strings_ht);
+  }
+  if (hash == 0) {
+    hash = hash_hash(str);
+  }
+  hashitem_T *hi = hash_lookup(&compl_strings_ht, str, len, hash);
+  if (HASHITEM_EMPTY(hi)) {
+    complstr_T *entry = xmalloc(CSE_OFF + len + 1);
+    entry->cse_count = 1;
+    xmemcpyz(entry->cse_str, str, len);
+    hash_add_item(&compl_strings_ht, hi, entry->cse_str, hash);
+  } else {
+    HI2CSE(hi)->cse_count++;
+  }
+}
+
 /// list used to store the compl_T which have the max score
 static compl_T **compl_best_matches = NULL;
 static int compl_num_bests = 0;
@@ -949,6 +987,8 @@ static int ins_compl_add(char *const str, int len, char *const fname, char *cons
   const Direction dir = (cdir == kDirectionNotSet ? compl_direction : cdir);
   int flags = flags_arg;
   bool inserted = false;
+  char *new_str = NULL;
+  hash_T str_hash = 0;  // hash of the match string, when not 0
 
   if (flags & CP_FAST) {
     fast_breakcheck();
@@ -967,23 +1007,45 @@ static int ins_compl_add(char *const str, int len, char *const fname, char *cons
   }
 
   // If the same match is already present, don't add it.
-  if (compl_first_match != NULL && !adup) {
-    match = compl_first_match;
-    do {
-      if (!match_at_original_text(match)
-          && strncmp(match->cp_str.data, str, (size_t)len) == 0
-          && ((int)match->cp_str.size <= len || match->cp_str.data[len] == NUL)) {
-        if (is_nearest_active() && score > 0 && score < match->cp_score) {
-          match->cp_score = score;
-        }
-        if (cptext_allocated) {
-          free_cptext(cptext);
-        }
-        xfree(commit_chars);
-        return NOTDONE;
+  if (compl_first_match != NULL && !adup && compl_strings_ht.ht_used > 0) {
+    // Use a stack buffer for the NUL-terminated key when it fits, so
+    // that rejecting a duplicate does not allocate memory.
+    char keybuf[128];
+    char *key;
+    if (len < (int)sizeof(keybuf)) {
+      memmove(keybuf, str, (size_t)len);
+      keybuf[len] = NUL;
+      key = keybuf;
+    } else {
+      new_str = xstrnsave(str, (size_t)len);
+      key = new_str;
+    }
+    str_hash = hash_hash(key);
+    hashitem_T *hi = hash_lookup(&compl_strings_ht, key, (size_t)len, str_hash);
+    if (!HASHITEM_EMPTY(hi)) {
+      if (is_nearest_active() && score > 0) {
+        // The duplicate may need its score updated, scan the
+        // matches to find it.
+        match = compl_first_match;
+        do {
+          if (!match_at_original_text(match)
+              && strncmp(match->cp_str.data, str, (size_t)len) == 0
+              && ((int)match->cp_str.size <= len || match->cp_str.data[len] == NUL)) {
+            if (score < match->cp_score) {
+              match->cp_score = score;
+            }
+            break;
+          }
+          match = match->cp_next;
+        } while (match != NULL && !is_first_match(match));
       }
-      match = match->cp_next;
-    } while (match != NULL && !is_first_match(match));
+      xfree(new_str);
+      if (cptext_allocated) {
+        free_cptext(cptext);
+      }
+      xfree(commit_chars);
+      return NOTDONE;
+    }
   }
 
   // Remove any popup menu before changing the list of matches.
@@ -993,7 +1055,8 @@ static int ins_compl_add(char *const str, int len, char *const fname, char *cons
   // Copy the values to the new match structure.
   match = xcalloc(1, sizeof(compl_T));
   match->cp_number = flags & CP_ORIGINAL_TEXT ? 0 : -1;
-  match->cp_str = cbuf_to_string(str, (size_t)len);
+  new_str = new_str == NULL ? xstrnsave(str, (size_t)len) : new_str;
+  match->cp_str = cbuf_as_string(new_str, (size_t)len);
   match->cp_commit_chars = commit_chars;
   match->cp_preselect = preselect;
   if (preselect && compl_preselect_match == NULL
@@ -1086,6 +1149,10 @@ static int ins_compl_add(char *const str, int len, char *const fname, char *cons
   }
   compl_curr_match = match;
 
+  if (!match_at_original_text(match)) {
+    compl_strings_add(match->cp_str.data, match->cp_str.size, str_hash);
+  }
+
   // Find the longest common string if still doing that.
   if (compl_get_longest && (flags & CP_ORIGINAL_TEXT) == 0 && !cot_fuzzy()
       && !ins_compl_preinsert_longest() && !ctrl_x_mode_thesaurus()) {
@@ -2117,6 +2184,20 @@ char *find_line_end(char *ptr)
 /// Free a completion item in the list
 static void ins_compl_item_free(compl_T *match)
 {
+  // Uncount the match string in the duplicate-check hashtab; the entry is
+  // only removed with its last match.  The hashtab is empty when it was
+  // already cleared as a whole by ins_compl_free().
+  if (compl_strings_ht.ht_used > 0 && match->cp_str.data != NULL
+      && !match_at_original_text(match)) {
+    hashitem_T *hi = hash_find(&compl_strings_ht, match->cp_str.data);
+    if (!HASHITEM_EMPTY(hi)) {
+      complstr_T *entry = HI2CSE(hi);
+      if (--entry->cse_count <= 0) {
+        hash_remove(&compl_strings_ht, hi);
+        xfree(entry);
+      }
+    }
+  }
   API_CLEAR_STRING(match->cp_str);
   // several entries may use the same fname, free it just once.
   if (match->cp_flags & CP_FREE_FNAME) {
@@ -2141,6 +2222,11 @@ static void ins_compl_free(void)
   ins_compl_del_pum();
   pum_clear();
 
+  // Free the duplicate-check hashtab entries all at once, then freeing
+  // the matches below does not need to uncount them one by one.
+  hash_clear_all(&compl_strings_ht, CSE_OFF);
+  hash_init(&compl_strings_ht);
+
   compl_curr_match = compl_first_match;
   do {
     compl_T *match = compl_curr_match;

test/old/testdir/test_ins_complete.vim

@@ -5997,7 +5997,9 @@ func Test_completetimeout_autocompletetimeout()
   set completetimeout=1
   call feedkeys("Gof\<C-N>\<F2>\<Esc>0", 'xt!')
   let match_count = len(b:matches->mapnew('v:val.word'))
-  call assert_true(match_count < 4000)
+  " How many matches are collected in 1 msec varies with machine speed, only
+  " check the timeout truncated the collection.
+  call assert_true(match_count < 60000)
 
   set completetimeout=1000
   call feedkeys("\<Esc>Sf\<C-N>\<F2>\<Esc>0", 'xt!')
@@ -6006,9 +6008,14 @@ func Test_completetimeout_autocompletetimeout()
 
   set autocomplete
   set autocompletetimeout=81
+  " Use enough long words that collecting all of them takes well over the
+  " timeout even on a fast machine.
+  let pad = repeat('y', 60)
+  call setline(1, map(range(200000), '"foo" . v:val . pad'))
   call feedkeys("\<Esc>Sf\<F2>\<Esc>0", 'xt!')
   let match_count = len(b:matches->mapnew('v:val.word'))
-  call assert_true(match_count < 50000)
+  " The timeout must have truncated the collection.
+  call assert_true(match_count < 200000)
 
   set complete& omnifunc& autocomplete& autocompletetimeout& completetimeout&
   bwipe!
@@ -6729,4 +6736,90 @@ func Test_complete_check_mapped_typed_key()
   unlet g:compl_iterations
 endfunc
 
+" Test for the duplicate check when adding completion matches
+func Test_ins_complete_dedup()
+  new
+  setl complete=.
+
+  " a word that occurs several times only results in one match
+  call setline(1, ['alpha beta alpha gamma', 'beta alpha delta beta', ''])
+  call cursor(3, 1)
+  call feedkeys("Aal\<C-N>\<C-R>=GetCompleteInfo()\<CR>\<C-E>\<Esc>", 'tx')
+  call assert_equal(['alpha'], g:compl_info.items->mapnew('v:val.word'))
+
+  " the duplicate check is case-sensitive
+  %delete _
+  call setline(1, ['Foo foo FOO fooBar Foo foo', ''])
+  call cursor(2, 1)
+  call feedkeys("Afo\<C-N>\<C-R>=GetCompleteInfo()\<CR>\<C-E>\<Esc>", 'tx')
+  call assert_equal(['foo', 'fooBar'], g:compl_info.items->mapnew('v:val.word'))
+
+  " with 'ignorecase' and 'infercase' case variants fold into one match
+  setl ignorecase infercase
+  %delete _
+  call setline(1, ['Word word WORD wordy Word', ''])
+  call cursor(2, 1)
+  call feedkeys("Awo\<C-N>\<C-R>=GetCompleteInfo()\<CR>\<C-E>\<Esc>", 'tx')
+  call assert_equal(['word', 'wordy'], g:compl_info.items->mapnew('v:val.word'))
+  setl noignorecase noinfercase
+
+  " duplicate dictionary entries only appear once; with 'ignorecase' case
+  " variants all match but stay separate matches
+  call writefile(['apple', 'apple', 'Apple', 'apricot', 'apricot', 'banana'],
+        \ 'Xcompldict', 'D')
+  setl dictionary=Xcompldict
+  set ignorecase
+  %delete _
+  call feedkeys("Aap\<C-X>\<C-K>\<C-R>=GetCompleteInfo()\<CR>\<C-E>\<Esc>", 'tx')
+  call assert_equal(['apple', 'Apple', 'apricot'], g:compl_info.items->mapnew('v:val.word'))
+  set noignorecase
+  setl dictionary&
+
+  " duplicate items passed to complete() are only added once
+  inoremap <buffer> <F5> <Cmd>call complete(1, ['dup', 'dup', 'uniq', 'dup'])<CR>
+  %delete _
+  call feedkeys("i\<F5>\<C-R>=GetCompleteInfo()\<CR>\<C-E>\<Esc>", 'tx')
+  call assert_equal(['dup', 'uniq'], g:compl_info.items->mapnew('v:val.word'))
+
+  " restarting a completion rebuilds the matches without duplicates
+  %delete _
+  call setline(1, ['echo edit eecho edit echo', ''])
+  call cursor(2, 1)
+  call feedkeys("Ae\<C-N>\<C-E>\<Esc>", 'tx')
+  call feedkeys("A\<C-N>\<C-R>=GetCompleteInfo()\<CR>\<C-E>\<Esc>", 'tx')
+  call assert_equal(['echo', 'edit', 'eecho'], g:compl_info.items->mapnew('v:val.word'))
+
+  " With "dup" matches from several sources, refreshing one source removes
+  " its duplicate but must not forget about the equal match of the other
+  " source: adding "dupword" again without "dup" is still a duplicate.
+  let g:dedup_calls = 0
+  func! DedupSrcA(findstart, base)
+    if a:findstart
+      return 0
+    endif
+    let g:dedup_calls += 1
+    if g:dedup_calls == 1
+      return #{words: [#{word: 'dupword', dup: 1}], refresh: 'always'}
+    endif
+    return #{words: [#{word: 'dupword'}], refresh: 'always'}
+  endfunc
+  func! DedupSrcB(findstart, base)
+    if a:findstart
+      return 0
+    endif
+    return #{words: [#{word: 'dupword', dup: 1}]}
+  endfunc
+  setl complete=FDedupSrcA,FDedupSrcB
+  %delete _
+  call feedkeys("Sdup\<C-N>\<BS>\<C-R>=GetCompleteInfo()\<CR>\<C-E>\<Esc>", 'tx')
+  call assert_equal(['dupword'], g:compl_info.items->mapnew('v:val.word'))
+  setl complete&
+  delfunc DedupSrcA
+  delfunc DedupSrcB
+  unlet g:dedup_calls
+
+  bwipe!
+  unlet g:compl_info
+endfunc
+
 " vim: shiftwidth=2 sts=2 expandtab nofoldenable