perf(column): only invalidate lines affected by added sign

5 files changed, 64 insertions, 76 deletions

diff --git a/src/nvim/buffer.c b/src/nvim/buffer.c
index 4d553cc91f..e6bedca232 100644
--- a/src/nvim/buffer.c
+++ b/src/nvim/buffer.c
@@ -1844,7 +1844,7 @@ buf_T *buflist_new(char *ffname_arg, char *sfname_arg, linenr_T lnum, int flags)
     buf = xcalloc(1, sizeof(buf_T));
     // init b: variables
     buf->b_vars = tv_dict_alloc();
-    buf->b_signcols.valid = false;
+    buf->b_signcols.sentinel = 0;
     init_var_dict(buf->b_vars, &buf->b_bufvar, VAR_SCOPE);
     buf_init_changedtick(buf);
   }
@@ -4026,88 +4026,64 @@ char *buf_spname(buf_T *buf)
   return NULL;
 }
 
-/// Invalidate the signcolumn if needed after deleting
-/// signs between line1 and line2 (inclusive).
-///
-/// @param buf   buffer to check
-/// @param line1 start of region being deleted
-/// @param line2 end of region being deleted
+/// Invalidate the signcolumn if needed after deleting a sign ranging from line1 to line2.
 void buf_signcols_del_check(buf_T *buf, linenr_T line1, linenr_T line2)
 {
-  if (!buf->b_signcols.valid) {
-    return;
-  }
-
-  if (!buf->b_signcols.sentinel) {
-    buf->b_signcols.valid = false;
-    return;
-  }
-
   linenr_T sent = buf->b_signcols.sentinel;
-
   if (sent >= line1 && sent <= line2) {
-    // Only invalidate when removing signs at the sentinel line.
-    buf->b_signcols.valid = false;
+    // When removed sign overlaps the sentinel line, entire buffer needs to be checked.
+    buf->b_signcols.sentinel = buf->b_signcols.size = 0;
   }
 }
 
-/// Re-calculate the signcolumn after adding a sign.
-///
-/// @param buf   buffer to check
-/// @param added sign being added
+/// Invalidate the signcolumn if needed after adding a sign ranging from line1 to line2.
 void buf_signcols_add_check(buf_T *buf, linenr_T line1, linenr_T line2)
 {
-  if (!buf->b_signcols.valid) {
-    return;
-  }
-
   if (!buf->b_signcols.sentinel) {
-    buf->b_signcols.valid = false;
     return;
   }
 
   linenr_T sent = buf->b_signcols.sentinel;
-
   if (sent >= line1 && sent <= line2) {
+    // If added sign overlaps sentinel line, increment without invalidating.
     if (buf->b_signcols.size == buf->b_signcols.max) {
       buf->b_signcols.max++;
     }
     buf->b_signcols.size++;
-    redraw_buf_later(buf, UPD_NOT_VALID);
     return;
   }
 
-  int signcols = decor_signcols(buf, line1 - 1, line2 - 1, SIGN_SHOW_MAX);
-
-  if (signcols > buf->b_signcols.size) {
-    buf->b_signcols.size = signcols;
-    buf->b_signcols.max = signcols;
-    redraw_buf_later(buf, UPD_NOT_VALID);
+  if (line1 < buf->b_signcols.invalid_top) {
+    buf->b_signcols.invalid_top = line1;
+  }
+  if (line2 > buf->b_signcols.invalid_bot) {
+    buf->b_signcols.invalid_bot = line2;
   }
 }
 
 int buf_signcols(buf_T *buf, int max)
 {
-  // The maximum can be determined from 'signcolumn' which is window scoped so
-  // need to invalidate signcols if the maximum is greater than the previous
-  // (valid) maximum.
-  if (buf->b_signcols.max && max > buf->b_signcols.max) {
-    buf->b_signcols.valid = false;
-  }
-
-  if (!buf->b_signcols.valid) {
-    buf->b_signcols.sentinel = 0;
-    int signcols = decor_signcols(buf, 0, (int)buf->b_ml.ml_line_count - 1, max);
-    // Check if we need to redraw
-    if (signcols != buf->b_signcols.size) {
-      buf->b_signcols.size = signcols;
-      redraw_buf_later(buf, UPD_NOT_VALID);
+  if (!buf->b_signs_with_text) {
+    buf->b_signcols.size = 0;
+  } else if (max <= 1 && buf->b_signs_with_text >= (size_t)max) {
+    buf->b_signcols.size = max;
+  } else {
+    linenr_T sent = buf->b_signcols.sentinel;
+    if (!sent || max > buf->b_signcols.max) {
+      // Recheck if the window scoped maximum 'signcolumn' is greater than the
+      // previous maximum or if there is no sentinel line yet.
+      buf->b_signcols.invalid_top = sent ? sent : 1;
+      buf->b_signcols.invalid_bot = sent ? sent : buf->b_ml.ml_line_count;
     }
 
-    buf->b_signcols.max = max;
-    buf->b_signcols.valid = true;
+    if (buf->b_signcols.invalid_bot) {
+      decor_validate_signcols(buf, max);
+    }
   }
 
+  buf->b_signcols.max = max;
+  buf->b_signcols.invalid_top = MAXLNUM;
+  buf->b_signcols.invalid_bot = 0;
   return buf->b_signcols.size;
 }
 
diff --git a/src/nvim/buffer_defs.h b/src/nvim/buffer_defs.h
index e0119d2add..e59539f900 100644
--- a/src/nvim/buffer_defs.h
+++ b/src/nvim/buffer_defs.h
@@ -706,9 +706,10 @@ struct file_buffer {
 
   struct {
     int size;                   // last calculated number of sign columns
-    bool valid;                 // calculated sign columns is valid
+    int max;                    // maximum value size is valid for.
     linenr_T sentinel;          // a line number which is holding up the signcolumn
-    int max;                    // Maximum value size is valid for.
+    linenr_T invalid_top;       // first invalid line number that needs to be checked
+    linenr_T invalid_bot;       // last invalid line number that needs to be checked
   } b_signcols;
 
   Terminal *terminal;           // Terminal instance associated with the buffer
diff --git a/src/nvim/decoration.c b/src/nvim/decoration.c
index 407b54d133..11204a1b31 100644
--- a/src/nvim/decoration.c
+++ b/src/nvim/decoration.c
@@ -791,22 +791,15 @@ DecorSignHighlight *decor_find_sign(DecorInline decor)
   }
 }
 
-// Get the maximum required amount of sign columns needed between row and
-// end_row.
-int decor_signcols(buf_T *buf, int row, int end_row, int max)
+// Increase the signcolumn size and update the sentinel line if necessary for
+// the invalidated range.
+void decor_validate_signcols(buf_T *buf, int max)
 {
-  if (max <= 1 && buf->b_signs_with_text >= (size_t)max) {
-    return max;
-  }
-
-  if (buf->b_signs_with_text == 0) {
-    return 0;
-  }
-
   int signcols = 0;  // highest value of count
+  int currow = buf->b_signcols.invalid_top - 1;
   // TODO(bfredl): only need to use marktree_itr_get_overlap once.
   // then we can process both start and end events and update state for each row
-  for (int currow = row; currow <= end_row; currow++) {
+  for (; currow < buf->b_signcols.invalid_bot; currow++) {
     MarkTreeIter itr[1];
     if (!marktree_itr_get_overlap(buf->b_marktree, currow, 0, itr)) {
       continue;
@@ -832,17 +825,16 @@ int decor_signcols(buf_T *buf, int row, int end_row, int max)
     }
 
     if (count > signcols) {
-      if (count > buf->b_signcols.size) {
+      if (count >= buf->b_signcols.size) {
+        buf->b_signcols.size = count;
         buf->b_signcols.sentinel = currow + 1;
       }
       if (count >= max) {
-        return max;
+        return;
       }
       signcols = count;
     }
   }
-
-  return signcols;
 }
 
 void decor_redraw_end(DecorState *state)
diff --git a/src/nvim/drawscreen.c b/src/nvim/drawscreen.c
index ea33c39178..6cc623cb72 100644
--- a/src/nvim/drawscreen.c
+++ b/src/nvim/drawscreen.c
@@ -606,10 +606,12 @@ int update_screen(void)
     }
 
     // Reset 'statuscolumn' if there is no dedicated signcolumn but it is invalid.
-    if (*wp->w_p_stc != NUL && !wp->w_buffer->b_signcols.valid && wp->w_minscwidth <= SCL_NO) {
+    if (*wp->w_p_stc != NUL && wp->w_minscwidth <= SCL_NO
+        && (wp->w_buffer->b_signcols.invalid_bot || !wp->w_buffer->b_signcols.sentinel)) {
       wp->w_nrwidth_line_count = 0;
       wp->w_valid &= ~VALID_WCOL;
       wp->w_redr_type = UPD_NOT_VALID;
+      wp->w_buffer->b_signcols.invalid_bot = 0;
     }
   }
 
@@ -618,11 +620,6 @@ int update_screen(void)
   screen_search_hl.rm.regprog = NULL;
 
   FOR_ALL_WINDOWS_IN_TAB(wp, curtab) {
-    // Validate b_signcols if there is no dedicated signcolumn but 'statuscolumn' is set.
-    if (*wp->w_p_stc != NUL && wp->w_minscwidth <= SCL_NO) {
-      buf_signcols(wp->w_buffer, 0);
-    }
-
     if (wp->w_redr_type == UPD_CLEAR && wp->w_floating && wp->w_grid_alloc.chars) {
       grid_invalidate(&wp->w_grid_alloc);
       wp->w_redr_type = UPD_NOT_VALID;
@@ -1213,6 +1210,7 @@ static void redraw_win_signcol(win_T *wp)
   wp->w_scwidth = win_signcol_count(wp);
   if (wp->w_scwidth != scwidth) {
     changed_line_abv_curs_win(wp);
+    redraw_later(wp, UPD_NOT_VALID);
   }
 }
 
diff --git a/test/functional/ui/sign_spec.lua b/test/functional/ui/sign_spec.lua
index adab184a4c..b12e79bd42 100644
--- a/test/functional/ui/sign_spec.lua
+++ b/test/functional/ui/sign_spec.lua
@@ -467,6 +467,27 @@ describe('Signs', function()
         {0:~                                                    }|
                                                              |
       ]])
+      -- should not increase size because sign with existing id is moved
+      command('sign place 4 line=1 name=pietSearch buffer=1')
+      screen:expect_unchanged()
+      command('sign unplace 4')
+      screen:expect([[
+        {1:>>>>>>}{6:  1 }a                                          |
+        {2:      }{6:  2 }b                                          |
+        {2:      }{6:  3 }c                                          |
+        {2:      }{6:  4 }^                                           |
+        {0:~                                                    }|
+        {0:~                                                    }|
+        {0:~                                                    }|
+        {0:~                                                    }|
+        {0:~                                                    }|
+        {0:~                                                    }|
+        {0:~                                                    }|
+        {0:~                                                    }|
+        {0:~                                                    }|
+                                                             |
+      ]])
+      command('sign place 4 line=1 name=pietSearch buffer=1')
       -- should keep the column at maximum size when signs are
       -- exceeding the maximum
       command('sign place 5 line=1 name=pietSearch buffer=1')